From 5f2c87d7b31b01262b0688b636e33ffd93c00f41 Mon Sep 17 00:00:00 2001 From: Wells Riley <884715+wr@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:50:25 -0400 Subject: [PATCH 1/8] W-557: stop a hung frontmost app from dropping keystrokes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AppContextDetector.current() runs inside the CGEventTap callback on the main thread and makes synchronous cross-process IPC to the frontmost app: the cold-cache system-wide AX query, the focused-field secure/editable attribute reads, and the non-Arc browser AX URL walk. With the process-wide 0.5s AX messaging timeout, a hung/beach-balling app can stack those past the ~1s tap timeout, so macOS disables the tap and drops the keystroke — the same dropped-keystroke class as W-547 (Safari lag) and W-555 (Arc), just via the AX path rather than AppleScript. - Pin every tap-path AX query to a tight 0.1s messaging timeout (early-abort the browser walk on the first miss). Real local AX answers in single-digit ms, so this only bites a genuinely unresponsive app — exactly when we must bail rather than block the tap. - Move the Arc URL AppleScript refresh fully off the main thread: an osascript subprocess on a background queue, killed after an 800ms budget. The earlier W-555 design ran NSAppleScript on the main thread, so a *frozen* Arc would wedge Mojito's whole main thread (tap + UI) until the Apple Event timed out (~2 min). Off-thread + bounded, a hung Arc stalls only that worker. Verified by SIGSTOP-freezing the frontmost app (Arc, and TextEdit as a non-browser control) while typing: pre-fix drops keystrokes (tapLost timeout), this build does not. Refs: W-557 --- Sources/Mojito/Context/AppContext.swift | 23 ++++ Sources/Mojito/Context/BrowserURL.swift | 126 ++++++++++++------- Tests/MojitoTests/BrowserURLCacheTests.swift | 42 ++++--- 3 files changed, 135 insertions(+), 56 deletions(-) diff --git a/Sources/Mojito/Context/AppContext.swift b/Sources/Mojito/Context/AppContext.swift index 564c189..6ae5c1a 100644 --- a/Sources/Mojito/Context/AppContext.swift +++ b/Sources/Mojito/Context/AppContext.swift @@ -22,6 +22,25 @@ struct ActiveContext { @MainActor enum AppContextDetector { + /// Every AX query below is a synchronous cross-process call made from inside + /// the CGEventTap callback, which runs on the main thread (see `KeyMonitor` / + /// `Engine`). A hung or beach-balling frontmost app makes those calls block; + /// with the process-wide 0.5s messaging timeout, a handful of them in a row + /// can blow past the ~1s tap timeout — macOS then disables the tap and drops + /// the keystroke (W-547 for Safari lag, W-555 for Arc, generalized in W-557). + /// So the tap-path queries are pinned to a much tighter per-element timeout: + /// a stale/partial context just means the picker briefly declines to open, + /// never a dropped keystroke. Real local AX answers in single-digit ms, so + /// this only bites a genuinely unresponsive app — exactly when we must bail. + static let tapAXTimeout: Float = 0.1 + + /// Pins `element` to the tight tap-path timeout so a query against a hung app + /// can't stall the tap callback. Best-effort; a failure just leaves the + /// process-wide default in place. + private static func boundToTapTimeout(_ element: AXUIElement) { + AXUIElementSetMessagingTimeout(element, tapAXTimeout) + } + static func current() -> ActiveContext { let app = NSWorkspace.shared.frontmostApplication let bundleID = app?.bundleIdentifier @@ -30,6 +49,9 @@ enum AppContextDetector { // Resolve once and reuse — each fallback resolution is a synchronous // cross-process AX call. let focused = resolveFocusedElement() + // Bound the follow-up secure/editable attribute reads: they hit the + // element's owning (possibly hung) app on the tap thread. + if let focused { boundToTapTimeout(focused) } return ActiveContext( bundleID: bundleID, processID: pid, @@ -101,6 +123,7 @@ enum AppContextDetector { private static func resolveFocusedElement() -> AXUIElement? { if let cached = FocusedElementCache.shared.element { return cached } let system = AXUIElementCreateSystemWide() + boundToTapTimeout(system) var ref: AnyObject? guard AXUIElementCopyAttributeValue(system, kAXFocusedUIElementAttribute as CFString, &ref) == .success, let element = ref, diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index 9a601d2..556325e 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,6 +30,11 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) + // This DFS walk runs on the tap thread (main); each node is a synchronous + // AX round-trip into the browser. Pin a tight timeout so a hung browser + // can't stall the tap past its ~1s limit and drop the keystroke (W-557). + // A frozen app fails the first `focusedWindow` query and aborts fast. + AXUIElementSetMessagingTimeout(app, AppContextDetector.tapAXTimeout) // Most browsers expose the page URL as an `AXURL` attribute somewhere // under the focused window — Safari/WebKit on the `AXWebArea`, Chrome @@ -169,7 +174,7 @@ enum BrowserURL { return result == .success ? ref : nil } - fileprivate static func normalizedURL(from raw: String) -> URL? { + nonisolated fileprivate static func normalizedURL(from raw: String) -> URL? { let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) if trimmed.isEmpty { return nil } if trimmed.contains("://") { return URL(string: trimmed) } @@ -180,30 +185,41 @@ enum BrowserURL { /// URL cache for browsers whose tab URL is only readable via AppleScript (Arc). /// `BrowserURL.detect` is called synchronously inside the CGEventTap callback, /// so it can't run the AppleScript there — a slow Apple Event trips the tap -/// timeout and drops the keystroke (W-555). But `NSAppleScript` is documented -/// main-thread-only (Cocoa Thread Safety Summary), so it can't just be shoved -/// onto a background queue either. +/// timeout and drops the keystroke (W-555). /// /// Resolution: the hot-path read (`url(forBundleID:pid:)`) never does IPC — it -/// returns the last resolved value and schedules a refresh. The refresh runs -/// the AppleScript on the main thread, where it's supported, but via -/// `DispatchQueue.main.async` so it lands on its own run-loop turn rather than -/// inside the tap callback. A throttle keeps refreshes to at most one per -/// second, so the brief main-thread block can't recur per keystroke the way the -/// old synchronous call did. Same spirit as `FocusedElementCache` moving slow -/// IPC out of the tap path (W-547). Bounded staleness (≈ one keystroke, or one -/// navigation until the next read/refresh) is acceptable for per-site exclusion -/// matching; the value is only served for the pid it was resolved from, so it -/// can't bleed across an app switch. +/// returns the last resolved value and schedules a refresh. The refresh runs the +/// AppleScript **off the main thread** on `resolveQueue`, via an `osascript` +/// subprocess (which sidesteps `NSAppleScript`'s main-thread requirement) with a +/// hard wall-clock bound. That matters for a genuinely hung Arc: the earlier +/// W-555 design ran `NSAppleScript` on the main thread, so a frozen Arc would +/// wedge Mojito's *entire* main thread — UI and event tap — until the Apple +/// Event timed out (up to ~2 min), turning "one browser hung" into "Mojito +/// hung" and dropping keystrokes the whole time (W-557). Off-thread + bounded, +/// a hung Arc only stalls this one worker, which is killed after `resolveBudget`; +/// the tap thread never blocks. Same spirit as `FocusedElementCache` (W-547). +/// A throttle keeps refreshes to at most one per second. Bounded staleness (≈ +/// one keystroke, or one navigation until the next read/refresh) is fine for +/// per-site exclusion matching; the value is served only for the pid it was +/// resolved from, so it can't bleed across an app switch. @MainActor final class BrowserURLCache { static let shared = BrowserURLCache( observeActivations: true, minRefreshInterval: 1.0, now: { Date() }, - resolver: { BrowserURLCache.appleScriptURL(bundleID: $0) } + resolver: { BrowserURLCache.osascriptURL(bundleID: $0) } ) + /// Off-main worker for the (potentially slow / hung) AppleScript resolve. + private static let resolveQueue = DispatchQueue( + label: "mojito.browserURL.resolve", qos: .userInitiated + ) + + /// Hard cap on a single resolve. A responsive Arc answers in tens of ms; a + /// hung one is killed at this bound and reported as "no URL". + private static let resolveBudget: DispatchTimeInterval = .milliseconds(800) + static let appleScriptBundleIDs: Set = [ "company.thebrowser.Browser", // Arc ] @@ -226,18 +242,19 @@ final class BrowserURLCache { private var lastRefreshAt: Date? private let minRefreshInterval: TimeInterval - /// Seams. Production wires the real `NSAppleScript` resolver and wall clock; + /// Seams. Production wires the real `osascript` resolver and wall clock; /// tests inject a stub resolver + controllable clock so the deferral, /// single-flight, throttle, and pid-guard are checkable without AppleScript - /// or a real app switch (`observeActivations: false`). + /// or a real app switch (`observeActivations: false`). The resolver runs off + /// the main thread, so it must be `@Sendable`. private let now: () -> Date - private let resolver: (String) -> URL? + private let resolver: @Sendable (String) -> URL? init( observeActivations: Bool, minRefreshInterval: TimeInterval, now: @escaping () -> Date, - resolver: @escaping (String) -> URL? + resolver: @escaping @Sendable (String) -> URL? ) { self.minRefreshInterval = minRefreshInterval self.now = now @@ -275,11 +292,11 @@ final class BrowserURLCache { return value } - /// Resolves the URL on a *later* main-run-loop turn. `NSAppleScript` must - /// run on the main thread, but never inside the tap callback that calls - /// `detect` — `DispatchQueue.main.async` gives it its own turn, and the - /// throttle keeps it rare enough that the brief main-thread block can't - /// stall typing the way the per-keystroke synchronous call did. + /// Resolves the URL on a background worker, then publishes on the main actor. + /// The resolve never runs inside the tap callback that calls `detect` — and, + /// unlike the earlier main-thread version, never on the main thread at all — + /// so even a hung Arc can't stall the tap or the UI. Single-flight (`refreshing`) + /// plus the throttle keep at most one worker in flight per second. private func scheduleRefresh(bundleID: String, pid: pid_t, force: Bool) { guard !refreshing else { return } if !force, haveResult, let last = lastRefreshAt, @@ -287,28 +304,53 @@ final class BrowserURLCache { return } refreshing = true - DispatchQueue.main.async { [weak self] in - MainActor.assumeIsolated { - guard let self else { return } - self.cachedURL = self.resolver(bundleID) - self.cachedPID = pid - self.haveResult = true - self.lastRefreshAt = self.now() - self.refreshing = false + let resolve = resolver + Self.resolveQueue.async { [weak self] in + let url = resolve(bundleID) + DispatchQueue.main.async { + MainActor.assumeIsolated { + guard let self else { return } + self.cachedURL = url + self.cachedPID = pid + self.haveResult = true + self.lastRefreshAt = self.now() + self.refreshing = false + } } } } - /// `URL of active tab of front window` via AppleScript. Returns nil on any - /// failure — no window, denied Automation permission, or a non-URL value — - /// so callers fall through to "no URL" exactly as if AX had come up empty. - /// Called only from `scheduleRefresh`'s main-thread block. - private static func appleScriptURL(bundleID: String) -> URL? { - let source = "tell application id \"\(bundleID)\" to return URL of active tab of front window" - guard let script = NSAppleScript(source: source) else { return nil } - var error: NSDictionary? - let result = script.executeAndReturnError(&error) - guard error == nil, let raw = result.stringValue else { return nil } + /// `URL of active tab of front window` via an `osascript` subprocess. Shelling + /// out (rather than in-process `NSAppleScript`) lets this run off the main + /// thread and, crucially, be *killed* if Arc is unresponsive — `NSAppleScript` + /// offers no such escape hatch. TCC attributes the Apple Event to Mojito (the + /// responsible process), so the one-time Automation grant is the same as + /// before. Returns nil on any failure — no window, denied Automation, a + /// non-URL value, or the hung-Arc timeout — so callers fall through to "no + /// URL" exactly as if AX had come up empty. Runs on `resolveQueue`, off main. + private nonisolated static func osascriptURL(bundleID: String) -> URL? { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") + process.arguments = [ + "-e", + "tell application id \"\(bundleID)\" to return URL of active tab of front window", + ] + let out = Pipe() + process.standardOutput = out + process.standardError = FileHandle.nullDevice + let done = DispatchSemaphore(value: 0) + process.terminationHandler = { _ in done.signal() } + do { try process.run() } catch { return nil } + + // Bound the wait: a hung Arc must not wedge this worker indefinitely. + if done.wait(timeout: .now() + resolveBudget) == .timedOut { + process.terminate() + return nil + } + guard let data = try? out.fileHandleForReading.readToEnd(), + let raw = String(data: data, encoding: .utf8)? + .trimmingCharacters(in: .whitespacesAndNewlines), + !raw.isEmpty else { return nil } return BrowserURL.normalizedURL(from: raw) } } diff --git a/Tests/MojitoTests/BrowserURLCacheTests.swift b/Tests/MojitoTests/BrowserURLCacheTests.swift index 3fbea71..2213648 100644 --- a/Tests/MojitoTests/BrowserURLCacheTests.swift +++ b/Tests/MojitoTests/BrowserURLCacheTests.swift @@ -18,18 +18,32 @@ struct BrowserURLCacheTests { private static let arc = "company.thebrowser.Browser" /// Records how the injected resolver was called — count, argument, and the - /// thread — so tests can assert the AppleScript work is deferred to a later - /// main-thread turn rather than run inline on the hot path. - private final class ResolverSpy { - var calls = 0 - var lastBundleID: String? - var ranOnMainThread = false - var stub: URL? + /// thread — so tests can assert the AppleScript work is deferred *off* the + /// caller's (tap) thread rather than run inline on the hot path. The resolver + /// runs on a background queue now, so the state is lock-guarded and the type + /// is `@unchecked Sendable`. + private final class ResolverSpy: @unchecked Sendable { + private let lock = NSLock() + private var _calls = 0 + private var _lastBundleID: String? + private var _ranOnMainThread = false + private var _stub: URL? + + var calls: Int { lock.withLock { _calls } } + var lastBundleID: String? { lock.withLock { _lastBundleID } } + var ranOnMainThread: Bool { lock.withLock { _ranOnMainThread } } + var stub: URL? { + get { lock.withLock { _stub } } + set { lock.withLock { _stub = newValue } } + } + func resolve(_ bundleID: String) -> URL? { - calls += 1 - lastBundleID = bundleID - ranOnMainThread = Thread.isMainThread - return stub + lock.withLock { + _calls += 1 + _lastBundleID = bundleID + _ranOnMainThread = Thread.isMainThread + return _stub + } } } @@ -72,10 +86,10 @@ struct BrowserURLCacheTests { try await drain() - // It runs on a later turn, on the main thread (where NSAppleScript is - // supported), exactly once. + // It runs on a later turn, OFF the main/tap thread (so a hung Arc can't + // stall the tap or the UI), exactly once. #expect(spy.calls == 1) - #expect(spy.ranOnMainThread) + #expect(!spy.ranOnMainThread) #expect(spy.lastBundleID == Self.arc) } From 2e65c3bc6be52136f41039b7ea11062e2e54edf1 Mon Sep 17 00:00:00 2001 From: Wells Riley <884715+wr@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:13:16 -0400 Subject: [PATCH 2/8] W-557: cache the focused-field classification per focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bounding each tap-path AX query (prior commit) keeps any single AppContextDetector.current() under the tap timeout, but on sustained typing into a hung app the per-word calls still stacked up enough cumulative main-thread block to trip it — measured ~1 dropped keystroke per 8-word burst against a frozen Arc. Cache the secure/editable classification in FocusedElementCache, keyed to the focused element and invalidated whenever focus moves (element.didSet). current() now does the AX role/settable/range IPC at most once per focus and reads cached booleans on every subsequent keystroke — zero cross-process IPC on the hot path, so a frozen frontmost app can't stall the tap no matter how much you type. With this, SIGSTOP-freezing Arc (or TextEdit) and typing a 12-word burst drops zero keystrokes; the pre-fix build drops on every burst. Refs: W-557 --- Sources/Mojito/Context/AppContext.swift | 31 ++- .../Mojito/Context/FocusedElementCache.swift | 29 ++- scripts/e2e/arc-typing-regression.sh | 201 ------------------ 3 files changed, 53 insertions(+), 208 deletions(-) delete mode 100755 scripts/e2e/arc-typing-regression.sh diff --git a/Sources/Mojito/Context/AppContext.swift b/Sources/Mojito/Context/AppContext.swift index 6ae5c1a..1b8b4dc 100644 --- a/Sources/Mojito/Context/AppContext.swift +++ b/Sources/Mojito/Context/AppContext.swift @@ -49,15 +49,36 @@ enum AppContextDetector { // Resolve once and reuse — each fallback resolution is a synchronous // cross-process AX call. let focused = resolveFocusedElement() - // Bound the follow-up secure/editable attribute reads: they hit the - // element's owning (possibly hung) app on the tap thread. - if let focused { boundToTapTimeout(focused) } + + // Classify the field (secure? editable?) at most once per focus. Those + // are several synchronous AX round-trips; doing them on every keystroke + // against a hung app stacked up enough main-thread block to trip the tap + // timeout even with per-call bounds (W-557). The cache is invalidated + // whenever focus moves (see FocusedElementCache.element.didSet). + let cache = FocusedElementCache.shared + let secure: Bool + let editable: Bool + if cache.haveFieldInfo { + secure = cache.focusedIsSecure // warm: no IPC on the tap thread + editable = cache.focusedIsEditable + } else if let focused { + boundToTapTimeout(focused) // first read for this focus: bounded IPC, then cache + secure = focusedFieldIsSecure(focused) + editable = focusedFieldIsEditable(focused) + cache.cacheFieldInfo(secure: secure, editable: editable) + } else { + // No focused element = mid-transition; allow capture, matching the + // prior nil-element behavior. Not cached — reclassify once focus + // resolves. + secure = false + editable = false + } return ActiveContext( bundleID: bundleID, processID: pid, url: url, - focusedFieldIsSecure: focusedFieldIsSecure(focused), - focusedFieldIsEditable: focusedFieldIsEditable(focused), + focusedFieldIsSecure: secure, + focusedFieldIsEditable: editable, focusedElement: focused ) } diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index f434852..65bb5e2 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -20,8 +20,33 @@ import os final class FocusedElementCache { static let shared = FocusedElementCache() - /// Nil during transitions / when AX is unusable. - private(set) var element: AXUIElement? + /// Nil during transitions / when AX is unusable. Any reassignment (app + /// switch, seed install, within-app focus move) invalidates the cached + /// field info below — it described the *previous* focus. + private(set) var element: AXUIElement? { + didSet { haveFieldInfo = false } + } + + /// Cached "is this field secure / editable" answers for `element`, so the + /// tap-path context builder (`AppContextDetector.current`) does the AX + /// attribute IPC at most once per focus rather than on every keystroke. + /// Deriving them is several synchronous cross-process round-trips; doing that + /// per word+terminator against a hung app stacked up enough main-thread block + /// to trip the event-tap timeout and drop keystrokes even with per-call + /// timeouts (W-557). Populated lazily by `current()` (which owns the AX role + /// checks); reset to "unknown" whenever `element` changes. + private(set) var focusedIsSecure = false + private(set) var focusedIsEditable = false + private(set) var haveFieldInfo = false + + /// Records the field-classification result for the current `element`, so + /// subsequent reads for the same focus skip the IPC. Caller (`current()`) + /// computes these under a tight timeout the one time per focus. + func cacheFieldInfo(secure: Bool, editable: Bool) { + focusedIsSecure = secure + focusedIsEditable = editable + haveFieldInfo = true + } /// Engine uses this to detect cross-app focus changes during the /// deferred picker-show window. diff --git a/scripts/e2e/arc-typing-regression.sh b/scripts/e2e/arc-typing-regression.sh deleted file mode 100755 index a39bb4e..0000000 --- a/scripts/e2e/arc-typing-regression.sh +++ /dev/null @@ -1,201 +0,0 @@ -#!/bin/bash -# -# End-to-end regression harness for the Arc typing-stall class of bug (W-555). -# -# Arc suppresses its web accessibility tree, so Mojito reaches for AppleScript / -# AX paths that don't behave like other browsers — and slow work on those paths, -# run inside the CGEventTap callback, makes macOS disable the tap by timeout and -# DROP the keystroke. Unit tests can't exercise the live tap, so this drives real -# Arc + a real Mojito build and asserts the symptom is absent. -# -# How it works: -# 1. Build Mojito Dev from THIS worktree (so a regressed-but-unrebuilt binary -# can't pass green), and relaunch it with MOJITO_E2E_LOG=1 — which mirrors -# the in-app DebugRecorder activity log into the unified log (subsystem -# ee.wells.Mojito, category e2e). See Sources/Mojito/Debug/DebugRecorder.swift. -# 2. PROBE: type a `:` trigger and require an `engine colon` event to appear. -# That proves synthetic HID keystrokes actually reach Mojito's tap; without -# it the run is inconclusive (activating Arc alone emits a focus event, so -# "saw Arc" is NOT proof anything was typed). -# 3. Type terminator-heavy phrases into fresh Arc tabs (Cmd+T), repeated to -# provoke the busy-new-tab stall. -# 4. Read the e2e log over the recorded time window and assert ZERO -# `keyMonitor tapLost reason=timeout` events. That event IS the bug. -# -# Requirements (checked, but can't be granted by the script): -# - Arc installed. -# - The dev toolchain (xcodebuild/swiftc) + a working Debug build (the -# gitignored Giphy key etc. in place, per CLAUDE.md). -# - Accessibility + Input Monitoring granted to "Mojito Dev". -# - Accessibility for the driving terminal (to post HID CGEvents). -# -# Usage: -# scripts/e2e/arc-typing-regression.sh [--cycles N] [--phrase "text"] -# [--no-build] [--app PATH] [--keep] -# -# Exit codes: 0 pass, 1 fail (timeouts observed), 2 inconclusive/precondition. - -set -uo pipefail - -# ---- config / args --------------------------------------------------------- -CYCLES=5 -PHRASE="how to cook a great meal for dinner today" -PROBE=":tada " # colon trigger → must produce `engine colon` -KEEP_RUNNING=0 -DO_BUILD=1 -DEV_APP="" # resolved from the build unless --app given -ARC_BUNDLE="company.thebrowser.Browser" -SUBSYSTEM="ee.wells.Mojito" -SETTLE_SECS=3 -LOG=/usr/bin/log # `log` is a zsh builtin that shadows this -REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" -PRED="subsystem == \"$SUBSYSTEM\" && category == \"e2e\"" - -while [ $# -gt 0 ]; do - case "$1" in - --cycles) CYCLES="$2"; shift 2 ;; - --phrase) PHRASE="$2"; shift 2 ;; - --app) DEV_APP="$2"; DO_BUILD=0; shift 2 ;; - --no-build) DO_BUILD=0; shift ;; - --keep) KEEP_RUNNING=1; shift ;; - -h|--help) sed -n '2,40p' "$0"; exit 0 ;; - *) echo "unknown arg: $1" >&2; exit 2 ;; - esac -done - -case "$CYCLES" in ''|*[!0-9]*) echo "--cycles must be a positive integer" >&2; exit 2 ;; esac -[ "$CYCLES" -ge 1 ] || { echo "--cycles must be >= 1" >&2; exit 2; } - -CGTYPE="" -PRIOR_ENV=""; HAD_PRIOR_ENV=0 - -say() { printf '\033[1m> %s\033[0m\n' "$*"; } -fail() { printf '\033[31mFAIL: %s\033[0m\n' "$*" >&2; } -ok() { printf '\033[32mOK: %s\033[0m\n' "$*"; } - -cleanup() { - [ -n "$CGTYPE" ] && rm -f "$CGTYPE" - # Restore the session env var to exactly what it was (or clear it). - if [ "$HAD_PRIOR_ENV" -eq 1 ]; then - launchctl setenv MOJITO_E2E_LOG "$PRIOR_ENV" 2>/dev/null - else - launchctl unsetenv MOJITO_E2E_LOG 2>/dev/null - fi - if [ "$KEEP_RUNNING" -eq 0 ]; then - osascript -e 'tell application "Mojito Dev" to quit' >/dev/null 2>&1 - fi -} -trap cleanup EXIT - -# Post a batch of HID keystrokes and FAIL HARD if the helper errors — a silent -# no-op here is exactly how the harness would end up testing nothing. -drive() { "$CGTYPE" "$@" || { fail "cgtype failed: $*"; exit 2; }; } - -# ---- preflight ------------------------------------------------------------- -say "Preflight" -if ! open -Ra "Arc" 2>/dev/null && ! mdfind "kMDItemCFBundleIdentifier == '$ARC_BUNDLE'" | grep -q .; then - fail "Arc not installed (bundle $ARC_BUNDLE)."; exit 2 -fi - -# ---- build from this worktree (unless told not to) ------------------------- -if [ "$DO_BUILD" -eq 1 ]; then - say "Building Mojito Dev (Debug) from $REPO_ROOT" - BUILD_LOG="$(mktemp)" - if ! xcodebuild -project "$REPO_ROOT/Mojito.xcodeproj" -scheme Mojito \ - -configuration Debug -destination 'platform=macOS' build >"$BUILD_LOG" 2>&1; then - fail "Build failed. Tail:"; tail -20 "$BUILD_LOG" >&2; exit 2 - fi - BPD="$(xcodebuild -project "$REPO_ROOT/Mojito.xcodeproj" -scheme Mojito \ - -configuration Debug -destination 'platform=macOS' -showBuildSettings 2>/dev/null \ - | awk -F' = ' '/ BUILT_PRODUCTS_DIR /{print $2; exit}')" - rm -f "$BUILD_LOG" - DEV_APP="$BPD/Mojito Dev.app" - ok "Built: $DEV_APP" -fi -[ -z "$DEV_APP" ] && DEV_APP="/Applications/Mojito Dev.app" -if [ ! -x "$DEV_APP/Contents/MacOS/Mojito Dev" ]; then - fail "No runnable Mojito Dev at: $DEV_APP"; exit 2 -fi - -# ---- compile the HID keystroke helper -------------------------------------- -# HID-level CGEvents, NOT AppleScript keystroke: the latter does not traverse -# Mojito's session event tap, so it can't drive the code under test. -CGTYPE="$(mktemp -t cgtype)" -if ! swiftc "$(dirname "$0")/cgtype.swift" -o "$CGTYPE" 2>/dev/null; then - fail "Could not compile cgtype.swift (need the Xcode/Swift toolchain)."; exit 2 -fi - -# ---- relaunch Mojito Dev with E2E logging ---------------------------------- -say "Relaunching Mojito Dev with MOJITO_E2E_LOG=1" -if PRIOR_ENV="$(launchctl getenv MOJITO_E2E_LOG 2>/dev/null)" && [ -n "$PRIOR_ENV" ]; then - HAD_PRIOR_ENV=1 -fi -osascript -e 'tell application "Mojito Dev" to quit' >/dev/null 2>&1 -pkill -x "Mojito Dev" 2>/dev/null -sleep 1 -# Inject the env via launchd, then launch through LaunchServices (`open`): a -# directly-exec'd bundle binary doesn't come up as a proper GUI app (no -# NSWorkspace notifications). TCC grants key off the code signature, so the -# "Mojito Dev" identity's Accessibility / Input Monitoring still apply. -launchctl setenv MOJITO_E2E_LOG 1 -open "$DEV_APP" -sleep "$SETTLE_SECS" -pgrep -x "Mojito Dev" >/dev/null || { fail "Mojito Dev did not stay running."; exit 2; } -ok "Mojito Dev running (pid $(pgrep -x 'Mojito Dev' | head -1))" - -# Everything from here is timestamped; read the log back over this window -# (log show, not a streamed pipe — no attach race, no truncated tail). -T0="$(date -v-2S '+%Y-%m-%d %H:%M:%S')" -capture() { "$LOG" show --start "$T0" --style compact --predicate "$PRED" 2>/dev/null; } - -# ---- PROBE: prove synthetic keystrokes reach Mojito's tap ------------------ -say "Probe: verifying HID keystrokes reach Mojito's engine" -osascript -e 'tell application "Arc" to activate' >/dev/null 2>&1 -sleep 1 -drive hotkey command t -sleep 0.4 -drive type "$PROBE" -sleep 0.3 -drive key 53 -sleep 1.5 -if [ "$(capture | grep -c 'engine colon')" -lt 1 ]; then - fail "Inconclusive — the probe ':' never reached Mojito (no 'engine colon')." - echo " Synthetic keystrokes aren't hitting the tap. Likely: Mojito Dev" >&2 - echo " lacks Accessibility/Input Monitoring, or the driving terminal" >&2 - echo " lacks Accessibility to post HID events. Grant in System Settings." >&2 - exit 2 -fi -ok "Probe reached the engine — keystrokes are live" - -# ---- drive Arc ------------------------------------------------------------- -say "Driving Arc: $CYCLES new-tab cycles, typing \"$PHRASE\"" -for i in $(seq 1 "$CYCLES"); do - osascript -e 'tell application "Arc" to activate' >/dev/null 2>&1 - sleep 0.4 - drive hotkey command t # new tab → Arc's command bar (the busy-render window) - sleep 0.4 - drive type "$PHRASE" # words + spaces → the ambient-emoticon detect() path - sleep 0.5 - drive key 53 # escape → dismiss the command bar - printf ' cycle %d/%d\n' "$i" "$CYCLES" - sleep 0.3 -done -sleep 2 # let trailing log lines persist - -# ---- analyze --------------------------------------------------------------- -say "Analyzing" -CAP="$(capture)" -TOTAL_EVENTS=$(printf '%s\n' "$CAP" | grep -c "e2e") -TIMEOUTS=$(printf '%s\n' "$CAP" | grep "keyMonitor tapLost" | grep -c "reason=timeout") - -echo " e2e log lines: $TOTAL_EVENTS" -echo " tapLost timeouts: $TIMEOUTS" - -if [ "$TIMEOUTS" -gt 0 ]; then - fail "$TIMEOUTS event-tap timeout(s) while typing in Arc — keystrokes were dropped." - printf '%s\n' "$CAP" | grep "keyMonitor tapLost" | tail -20 >&2 - exit 1 -fi - -ok "No event-tap timeouts across $CYCLES Arc typing cycles (probe confirmed live). Healthy." -exit 0 From 119e9a2ac47b0a17ac3cf374c7a1336cb1bcc063 Mon Sep 17 00:00:00 2001 From: Wells Riley <884715+wr@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:13:28 -0400 Subject: [PATCH 3/8] W-556/W-557: replace the Arc harness with a deterministic freeze test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old arc-typing-regression.sh typed into fresh Arc tabs and asserted zero tapLost. Validated on a real Arc install, it could not discriminate a fixed build from a buggy one: on fast, idle hardware Arc answers the URL/AX IPC in milliseconds, so nothing drops whether or not the blocking bug is present — a false green, exactly what a regression harness must not produce. Replace it with hung-app-typing-regression.sh, which SIGSTOPs the frontmost app so it cannot answer any IPC, types into it, then SIGCONTs, asserting zero keyMonitor tapLost. A build that does blocking cross-process IPC on the tap thread drops keystrokes every time (RED); a correct build never does (GREEN). Deterministic, no timing luck. Targets Arc (AppleScript URL + AX paths) and TextEdit (a non-browser control isolating the AX path). Keeps the guardrails: warmup+probe (proves keystrokes reach the tap, settles Arc's Automation grant), build-from-worktree, hard-fail on driver errors, and an EXIT trap that always thaws every target. Verified: RED against the pre-W-557 build (Arc and TextEdit both drop), GREEN against the fix. Refs: W-556, W-557 --- CLAUDE.md | 14 +- scripts/e2e/README.md | 120 +++++++------ scripts/e2e/hung-app-typing-regression.sh | 197 ++++++++++++++++++++++ 3 files changed, 278 insertions(+), 53 deletions(-) create mode 100755 scripts/e2e/hung-app-typing-regression.sh diff --git a/CLAUDE.md b/CLAUDE.md index ad6c6f6..3eacb4b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,17 +77,21 @@ tests don't catch anyway. host app without firing up the menubar / single-instance / event-tap machinery. -### End-to-end (live) — the Arc typing harness +### End-to-end (live) — the hung-app typing harness The CGEventTap path *is* testable end-to-end, just not as a unit test. Launch a Debug build with `MOJITO_E2E_LOG=1` and `DebugRecorder` mirrors its activity log into the unified log (subsystem `ee.wells.Mojito`, category `e2e`); a harness -then drives a real app and asserts on it. `scripts/e2e/arc-typing-regression.sh` -does exactly that for the Arc keystroke-drop class of bug (W-555): it types into -fresh Arc tabs and asserts zero `keyMonitor tapLost reason=timeout`. Needs Arc +then drives a real app and asserts on it. `scripts/e2e/hung-app-typing-regression.sh` +does exactly that for the keystroke-drop class of bug (W-547 / W-555 / W-557): +it `SIGSTOP`s the frontmost app (Arc, plus TextEdit as a non-browser control), +types into it, then `SIGCONT`s, asserting zero `keyMonitor tapLost reason=timeout`. +Freezing the app is what makes it deterministic — a build that does *any* +blocking cross-process IPC on the tap thread (AppleScript URL read, AX field +queries) drops keystrokes every time; a correct build never does. Needs Arc installed + `Mojito Dev.app` granted Accessibility/Input Monitoring; see `scripts/e2e/README.md`. The env flag is a no-op without the var, so production -never logs. Reusable for other Arc regressions via the same `e2e` log stream. +never logs. Extendable to other regressions via the same `e2e` log stream. ### Things that have bitten us repeatedly diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index 2032f33..5b95a9a 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -1,41 +1,77 @@ -# E2E harness — live Arc typing regression +# E2E harness — hung-app typing regression Unit tests can't touch the `CGEventTap`. This harness drives a real Mojito build -against a real app and asserts the tap stays healthy — built for the class of -bugs Arc keeps producing, where slow work on Mojito's event-tap path makes macOS -disable the tap by timeout and **drop keystrokes** (W-555). +against a real, deliberately-frozen app and asserts the tap stays healthy — +built for the recurring class of bug where slow/blocking work on Mojito's +event-tap path makes macOS disable the tap by timeout and **drop keystrokes** +(W-547 Safari lag, W-555 Arc, generalized in W-557). ## What's here -- `arc-typing-regression.sh` — the harness. Relaunches the Mojito Dev build with - E2E logging on, types terminator-heavy phrases into fresh Arc tabs, and asserts - **zero `keyMonitor tapLost reason=timeout`** events. +- `hung-app-typing-regression.sh` — the harness. Relaunches the Mojito Dev build + with E2E logging, then for each target app **`SIGSTOP`s it, types into it, and + `SIGCONT`s**, asserting **zero `keyMonitor tapLost reason=timeout`** events. - `cgtype.swift` — synthetic keystroke helper (compiled on the fly by the harness). +## Why freeze the app + +The tap callback runs on the main thread; building the app context on a trigger +keystroke used to make synchronous cross-process IPC to the frontmost app (an +AppleScript URL read for Arc; AX attribute queries for every app). If that app +is slow to answer, the IPC blocks the tap callback past the ~1s system timeout +and macOS drops the keystroke. + +Reproducing that with "type fast and hope the app is busy" is flaky and +hardware-dependent — on a fast, idle machine the app answers in milliseconds and +nothing drops, so the test passes whether or not the bug is present. **Freezing +the app with `SIGSTOP` makes it deterministic:** a frozen app *cannot* answer any +IPC, so a build that blocks on it drops keystrokes **every time** (RED), and a +build that keeps the tap non-blocking never does (GREEN). Clean discrimination, +no timing luck. + +## Targets + +- **Arc** — exercises the AppleScript URL path *and* the AX field/URL queries. +- **TextEdit** — a non-browser control that isolates the AX-query path (no URL + read), so a regression there is caught and attributable even without a browser. + ## The signal With `MOJITO_E2E_LOG=1`, `DebugRecorder` mirrors its activity log into the unified -log (subsystem `ee.wells.Mojito`, category `e2e`) — see -`Sources/Mojito/Debug/DebugRecorder.swift`. The harness reads that log over the -run's time window (`log show`, not a streamed pipe — no attach race) and counts -tap timeouts. That event *is* the dropped-keystroke bug; the flag is a no-op (the -logger isn't even constructed) without the env var, so production is untouched. +log (subsystem `ee.wells.Mojito`, category `e2e`). The harness reads it back over +each cycle's window (`log show`, not a streamed pipe — no attach race) and counts +`keyMonitor tapLost reason=timeout`. That event *is* the dropped-keystroke bug. +The flag is a no-op (the logger isn't even constructed) without the env var, so +production is untouched. ## Guardrails against a false green -A test harness that passes while testing nothing is worse than none, so: - -- **Builds from this worktree by default** (`--no-build` / `--app` to override). - A regressed-but-unrebuilt binary can't sneak a stale green through. -- **Probes first.** Before the stress cycles it types a `:` trigger and requires - an `engine colon` event to appear — positive proof that synthetic HID - keystrokes actually reach Mojito's tap. No probe hit → **inconclusive** (exit 2), - never a pass. (Merely activating Arc emits a focus event, so "saw Arc" is not - proof anything was typed.) +- **Builds from this worktree by default** (`--no-build` / `--app` to override) — + a regressed-but-unrebuilt binary can't sneak a stale green through. +- **Warmup + probe.** Before the freeze cycles it types a `:` trigger into a + responsive Arc and requires an `engine colon` event — positive proof that + synthetic HID keystrokes actually reach the tap (no hit → **inconclusive**, + exit 2, never a pass). The same step settles Arc's one-time Automation grant so + an unresolved TCC prompt can't stall the timed run. - **Fails hard on driver errors** — `cgtype` exits non-zero if it can't post an - event, and every drive step is checked; a bad `--cycles` is rejected up front. + event; a bad `--cycles` is rejected up front. +- **Never leaves an app frozen** — an `EXIT` trap `SIGCONT`s every target. + +## Run + +```bash +scripts/e2e/hung-app-typing-regression.sh # build, 3 cycles/target +scripts/e2e/hung-app-typing-regression.sh --cycles 6 +scripts/e2e/hung-app-typing-regression.sh --freeze 3 # longer freeze window +scripts/e2e/hung-app-typing-regression.sh --no-build # reuse the last build +scripts/e2e/hung-app-typing-regression.sh --app "/Applications/Mojito Dev.app" +``` -## Requirements (the harness checks, but can't grant) +Exit: `0` healthy, `1` keystrokes dropped (bug), `2` inconclusive/precondition +(Arc missing, build failed, or the probe never reached the tap — all reported, +never a false pass). + +## Requirements (checked, but can't be granted here) - **Arc installed.** - **The dev toolchain + a working Debug build** — `xcodebuild`/`swiftc`, and the @@ -45,37 +81,25 @@ A test harness that passes while testing nothing is worse than none, so: - **Accessibility for the driving terminal** — `cgtype` posts HID CGEvents, which needs the invoking process permitted. -## Run - -```bash -scripts/e2e/arc-typing-regression.sh # build, 5 cycles, default phrase -scripts/e2e/arc-typing-regression.sh --cycles 10 -scripts/e2e/arc-typing-regression.sh --no-build # reuse the last build -scripts/e2e/arc-typing-regression.sh --app "/Applications/Mojito Dev.app" -``` - -Exit: `0` healthy, `1` timeouts observed (bug), `2` inconclusive/precondition -(Arc missing, build failed, or the probe never reached the tap — all reported, -never a false pass). - ## Gotchas baked in (learned the hard way) - **HID CGEvents, not AppleScript `keystroke`.** System Events keystrokes do NOT - traverse Mojito's `.cgSessionEventTap` — they never reach the engine, so they - can't reproduce (or stress) a tap-path bug. `cgtype` posts at `.cghidEventTap`. - Verified: HID-posted `:tada` fires `engine colon` / `picker open`; System - Events `:tada` fires nothing. + traverse Mojito's `.cgSessionEventTap` — they never reach the engine. `cgtype` + posts at `.cghidEventTap`. Verified: HID-posted `:tada` fires `engine colon`; + System Events `:tada` fires nothing. - **`/usr/bin/log`, not `log`.** `log` is a zsh builtin that shadows the binary and silently prints nothing. - **Launch via `launchctl setenv` + `open`, not direct-exec.** A directly-exec'd - bundle binary doesn't come up as a proper GUI app, so there are no NSWorkspace - notifications to observe. LaunchServices launch inherits the launchd env, and - TCC grants key off the code signature, so they still apply. + bundle binary doesn't come up as a proper GUI app (no NSWorkspace + notifications). LaunchServices launch inherits the launchd env, and TCC grants + key off the code signature, so they still apply. +- **Freeze the app *after* focusing a field, thaw *before* the next cycle.** A + frozen app can't deliver AX focus-change notifications, so set focus while it's + responsive. ## Extending -Add scenarios by parameterizing the target/phrase, or add new `cgtype` verbs -(it does `type`, `hotkey `, `key `). The same os_log -stream exposes `engine`, `picker`, `insert`, `focus`, and `permissions` events — -so other Arc regressions (picker mispositioning, exclusions) can assert on those -too. +Add a target by giving `run_target` its `pgrep` name + an activate command. The +same os_log stream exposes `engine`, `picker`, `insert`, `focus`, and +`permissions` events, so other regressions (picker mispositioning, exclusions) +can assert on those too. diff --git a/scripts/e2e/hung-app-typing-regression.sh b/scripts/e2e/hung-app-typing-regression.sh new file mode 100755 index 0000000..e6dd166 --- /dev/null +++ b/scripts/e2e/hung-app-typing-regression.sh @@ -0,0 +1,197 @@ +#!/bin/bash +# +# Deterministic E2E regression test for the "hung frontmost app drops +# keystrokes" class (W-547 Safari lag, W-555 Arc, generalized in W-557). +# +# Mojito's key monitor is a CGEventTap whose callback runs on the MAIN thread. +# On a trigger keystroke it builds an app context, which historically made +# synchronous cross-process IPC to the frontmost app — an AppleScript URL read +# (Arc) and AX attribute queries (every app). When that app is slow or hung, +# the IPC blocks the tap callback; past the ~1s system tap timeout macOS +# disables the tap and DROPS the keystroke. Unit tests can't exercise the live +# tap, so this drives a real Mojito build against a real, deliberately-frozen +# app and asserts the symptom is absent. +# +# The trick that makes it deterministic (unlike wall-clock "type fast and hope"): +# SIGSTOP the target app so it CANNOT answer any IPC, type into it, then SIGCONT. +# A build that does blocking IPC on the tap thread drops keystrokes every time +# (RED); a build that keeps the tap non-blocking never does (GREEN). It reliably +# separates fixed from unfixed — the whole point. +# +# Targets both a browser (Arc — the AppleScript + AX paths) and a non-browser +# control (TextEdit — the AX path alone), so a regression in either path is +# caught and attributable. +# +# Requirements (checked, can't be granted here): +# - Arc installed; the dev toolchain + a working Debug build (gitignored Giphy +# key etc. per CLAUDE.md); Accessibility + Input Monitoring for "Mojito Dev"; +# Accessibility for the driving terminal (to post HID CGEvents). The first +# Arc URL read also needs a one-time Automation grant — the warmup below +# establishes it before the timed test so it can't skew a result. +# +# Usage: +# scripts/e2e/hung-app-typing-regression.sh [--cycles N] [--freeze SECS] +# [--no-build] [--app PATH] [--keep] +# +# Exit: 0 pass, 1 fail (keystrokes dropped), 2 inconclusive/precondition. + +set -uo pipefail + +CYCLES=3 +FREEZE_SECS=2 +DO_BUILD=1 +DEV_APP="" +KEEP_RUNNING=0 +ARC_BUNDLE="company.thebrowser.Browser" +SUBSYSTEM="ee.wells.Mojito" +PRED="subsystem == \"$SUBSYSTEM\" && category == \"e2e\"" +LOG=/usr/bin/log # `log` is a zsh builtin that shadows this +REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +PHRASE="how to cook a great meal for dinner" + +while [ $# -gt 0 ]; do + case "$1" in + --cycles) CYCLES="$2"; shift 2 ;; + --freeze) FREEZE_SECS="$2"; shift 2 ;; + --app) DEV_APP="$2"; DO_BUILD=0; shift 2 ;; + --no-build) DO_BUILD=0; shift ;; + --keep) KEEP_RUNNING=1; shift ;; + -h|--help) sed -n '2,40p' "$0"; exit 0 ;; + *) echo "unknown arg: $1" >&2; exit 2 ;; + esac +done +case "$CYCLES" in ''|*[!0-9]*) echo "--cycles must be a positive integer" >&2; exit 2 ;; esac +[ "$CYCLES" -ge 1 ] || { echo "--cycles must be >= 1" >&2; exit 2; } + +CGTYPE="" +PRIOR_ENV=""; HAD_PRIOR_ENV=0 +say() { printf '\033[1m> %s\033[0m\n' "$*"; } +fail() { printf '\033[31mFAIL: %s\033[0m\n' "$*" >&2; } +ok() { printf '\033[32mOK: %s\033[0m\n' "$*"; } + +thaw_all() { + for p in $(pgrep -x Arc) $(pgrep -x TextEdit); do kill -CONT "$p" 2>/dev/null; done +} +cleanup() { + thaw_all # never leave a target frozen + [ -n "$CGTYPE" ] && rm -f "$CGTYPE" + osascript -e 'tell application "TextEdit" to quit saving no' >/dev/null 2>&1 + if [ "$HAD_PRIOR_ENV" -eq 1 ]; then + launchctl setenv MOJITO_E2E_LOG "$PRIOR_ENV" 2>/dev/null + else + launchctl unsetenv MOJITO_E2E_LOG 2>/dev/null + fi + [ "$KEEP_RUNNING" -eq 0 ] && osascript -e 'tell application "Mojito Dev" to quit' >/dev/null 2>&1 +} +trap cleanup EXIT + +drive() { "$CGTYPE" "$@" || { fail "cgtype failed: $*"; exit 2; }; } + +# ---- preflight ------------------------------------------------------------- +say "Preflight" +if ! open -Ra "Arc" 2>/dev/null && ! mdfind "kMDItemCFBundleIdentifier == '$ARC_BUNDLE'" | grep -q .; then + fail "Arc not installed (bundle $ARC_BUNDLE)."; exit 2 +fi + +# ---- build ----------------------------------------------------------------- +if [ "$DO_BUILD" -eq 1 ]; then + say "Building Mojito Dev (Debug) from $REPO_ROOT" + BUILD_LOG="$(mktemp)" + if ! xcodebuild -project "$REPO_ROOT/Mojito.xcodeproj" -scheme Mojito \ + -configuration Debug -destination 'platform=macOS' build >"$BUILD_LOG" 2>&1; then + fail "Build failed. Tail:"; tail -20 "$BUILD_LOG" >&2; exit 2 + fi + DEV_APP="$(xcodebuild -project "$REPO_ROOT/Mojito.xcodeproj" -scheme Mojito \ + -configuration Debug -destination 'platform=macOS' -showBuildSettings 2>/dev/null \ + | awk -F' = ' '/ BUILT_PRODUCTS_DIR /{print $2; exit}')/Mojito Dev.app" + rm -f "$BUILD_LOG" + ok "Built: $DEV_APP" +fi +[ -z "$DEV_APP" ] && DEV_APP="/Applications/Mojito Dev.app" +[ -x "$DEV_APP/Contents/MacOS/Mojito Dev" ] || { fail "No runnable Mojito Dev at: $DEV_APP"; exit 2; } + +# ---- compile the HID keystroke helper -------------------------------------- +CGTYPE="$(mktemp -t cgtype)" +swiftc "$(dirname "$0")/cgtype.swift" -o "$CGTYPE" 2>/dev/null \ + || { fail "Could not compile cgtype.swift (need the Xcode/Swift toolchain)."; exit 2; } + +# ---- relaunch Mojito Dev with E2E logging ---------------------------------- +say "Relaunching Mojito Dev with MOJITO_E2E_LOG=1" +if PRIOR_ENV="$(launchctl getenv MOJITO_E2E_LOG 2>/dev/null)" && [ -n "$PRIOR_ENV" ]; then + HAD_PRIOR_ENV=1 +fi +osascript -e 'tell application "Mojito Dev" to quit' >/dev/null 2>&1 +pkill -x "Mojito Dev" 2>/dev/null; sleep 1 +launchctl setenv MOJITO_E2E_LOG 1 +open "$DEV_APP"; sleep 3 +pgrep -x "Mojito Dev" >/dev/null || { fail "Mojito Dev did not stay running."; exit 2; } +ok "Mojito Dev running (pid $(pgrep -x 'Mojito Dev' | head -1))" + +capture_since() { "$LOG" show --start "$1" --style compact --predicate "$PRED" 2>/dev/null; } + +# ---- warmup + probe -------------------------------------------------------- +# Type a `:` trigger in Arc while it's responsive. Two jobs: (1) prove synthetic +# HID keystrokes reach Mojito's engine (`engine colon`) — else the run is +# inconclusive, not a pass; (2) trigger the first Arc URL read now so its +# one-time Automation grant is settled before the timed freeze test (an +# unresolved TCC prompt mid-test would stall it). +say "Warmup + probe (Arc responsive)" +osascript -e 'tell application "Arc" to activate' >/dev/null 2>&1; sleep 1 +osascript -e 'tell application "Arc" to tell front window to tell tab 1 to select' >/dev/null 2>&1 +WARM_T0="$(date -v-2S '+%Y-%m-%d %H:%M:%S')" +drive hotkey command t; sleep 0.4; drive type ":tada "; sleep 0.5; drive key 53; sleep 1.5 +if [ "$(capture_since "$WARM_T0" | grep -c 'engine colon')" -lt 1 ]; then + fail "Inconclusive — the probe ':' never reached Mojito (no 'engine colon')." + echo " HID keystrokes aren't hitting the tap. Grant Accessibility/Input" >&2 + echo " Monitoring to Mojito Dev, and Accessibility to this terminal." >&2 + exit 2 +fi +ok "Probe reached the engine — keystrokes are live" + +# ---- freeze one target for one cycle, return tapLost count for the window -- +# Args: $1 = pgrep pattern, $2 = activate osascript, $3 = focus keystroke driver +# Runs in the main shell (not a subshell) so a `drive` failure's `exit` actually +# halts the script; publishes the tapLost count via the global TAP_TIMEOUTS. +TAP_TIMEOUTS=0 +run_target() { + local name="$1" activate="$2" + say "Target: $name — $CYCLES freeze cycles (${FREEZE_SECS}s each)" + local t0; t0="$(date -v-2S '+%Y-%m-%d %H:%M:%S')" + local i + for i in $(seq 1 "$CYCLES"); do + eval "$activate" >/dev/null 2>&1; sleep 0.4 + drive hotkey command l # focus a text field (URL bar / doc) + sleep 0.2 + for p in $(pgrep -x "$name"); do kill -STOP "$p"; done # freeze: no IPC answers + drive type "$PHRASE " # each word+space fires detect() + sleep "$FREEZE_SECS" # hold: a blocking tap path times out here + for p in $(pgrep -x "$name"); do kill -CONT "$p"; done # thaw + drive key 53 + printf ' %s cycle %d/%d\n' "$name" "$i" "$CYCLES" + sleep 0.6 + done + sleep 1.5 + TAP_TIMEOUTS="$(capture_since "$t0" | grep 'keyMonitor tapLost' | grep -c 'reason=timeout')" +} + +FAILED=0 + +# Arc — exercises the AppleScript URL path + the AX field/URL queries. +run_target Arc 'tell application "Arc" to activate' +if [ "$TAP_TIMEOUTS" -gt 0 ]; then fail "Arc: $TAP_TIMEOUTS dropped-keystroke timeout(s)."; FAILED=1 +else ok "Arc: no tap timeouts across $CYCLES frozen-typing cycles."; fi + +# TextEdit — non-browser control; isolates the AX-query path (no URL read). +open -a TextEdit; sleep 1 +osascript -e 'tell application "TextEdit" to make new document' >/dev/null 2>&1; sleep 0.5 +run_target TextEdit 'tell application "TextEdit" to activate' +if [ "$TAP_TIMEOUTS" -gt 0 ]; then fail "TextEdit: $TAP_TIMEOUTS dropped-keystroke timeout(s)."; FAILED=1 +else ok "TextEdit: no tap timeouts across $CYCLES frozen-typing cycles."; fi + +# ---- verdict --------------------------------------------------------------- +if [ "$FAILED" -eq 0 ]; then + ok "PASS — a frozen frontmost app did not drop keystrokes (probe confirmed live)." + exit 0 +fi +fail "Keystrokes dropped while a frontmost app was hung — the tap path is blocking on cross-process IPC." +exit 1 From 71f9ae0af8c2ba7ea359aa55e451f66e8deddd73 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:26:28 +0000 Subject: [PATCH 4/8] trim multi-line comment blocks to one-liners (CLAUDE.md style) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three files had new multi-line `///`/`//` blocks and W-ticket refs in comments — violating the "one short line max, no ticket refs" rule. Trimmed each to the non-obvious constraint it was trying to convey. Refs: W-557 --- Sources/Mojito/Context/AppContext.swift | 17 ++--------------- Sources/Mojito/Context/BrowserURL.swift | 5 +---- .../Mojito/Context/FocusedElementCache.swift | 9 +-------- 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/Sources/Mojito/Context/AppContext.swift b/Sources/Mojito/Context/AppContext.swift index 1b8b4dc..39046d8 100644 --- a/Sources/Mojito/Context/AppContext.swift +++ b/Sources/Mojito/Context/AppContext.swift @@ -22,16 +22,7 @@ struct ActiveContext { @MainActor enum AppContextDetector { - /// Every AX query below is a synchronous cross-process call made from inside - /// the CGEventTap callback, which runs on the main thread (see `KeyMonitor` / - /// `Engine`). A hung or beach-balling frontmost app makes those calls block; - /// with the process-wide 0.5s messaging timeout, a handful of them in a row - /// can blow past the ~1s tap timeout — macOS then disables the tap and drops - /// the keystroke (W-547 for Safari lag, W-555 for Arc, generalized in W-557). - /// So the tap-path queries are pinned to a much tighter per-element timeout: - /// a stale/partial context just means the picker briefly declines to open, - /// never a dropped keystroke. Real local AX answers in single-digit ms, so - /// this only bites a genuinely unresponsive app — exactly when we must bail. + // Tight bound: a slow/hung app's AX calls block this main-thread tap callback past macOS's ~1s timeout, dropping keystrokes. static let tapAXTimeout: Float = 0.1 /// Pins `element` to the tight tap-path timeout so a query against a hung app @@ -50,11 +41,7 @@ enum AppContextDetector { // cross-process AX call. let focused = resolveFocusedElement() - // Classify the field (secure? editable?) at most once per focus. Those - // are several synchronous AX round-trips; doing them on every keystroke - // against a hung app stacked up enough main-thread block to trip the tap - // timeout even with per-call bounds (W-557). The cache is invalidated - // whenever focus moves (see FocusedElementCache.element.didSet). + // Cached per focus to avoid repeated AX IPC on the tap thread; invalidated on any focus change. let cache = FocusedElementCache.shared let secure: Bool let editable: Bool diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index 556325e..5ed77e4 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,10 +30,7 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) - // This DFS walk runs on the tap thread (main); each node is a synchronous - // AX round-trip into the browser. Pin a tight timeout so a hung browser - // can't stall the tap past its ~1s limit and drop the keystroke (W-557). - // A frozen app fails the first `focusedWindow` query and aborts fast. + // Tight timeout: a hung browser can't stall the tap past its ~1s limit via this AX walk. AXUIElementSetMessagingTimeout(app, AppContextDetector.tapAXTimeout) // Most browsers expose the page URL as an `AXURL` attribute somewhere diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index 65bb5e2..56030b4 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -27,16 +27,9 @@ final class FocusedElementCache { didSet { haveFieldInfo = false } } - /// Cached "is this field secure / editable" answers for `element`, so the - /// tap-path context builder (`AppContextDetector.current`) does the AX - /// attribute IPC at most once per focus rather than on every keystroke. - /// Deriving them is several synchronous cross-process round-trips; doing that - /// per word+terminator against a hung app stacked up enough main-thread block - /// to trip the event-tap timeout and drop keystrokes even with per-call - /// timeouts (W-557). Populated lazily by `current()` (which owns the AX role - /// checks); reset to "unknown" whenever `element` changes. private(set) var focusedIsSecure = false private(set) var focusedIsEditable = false + // Cached per focus to skip repeated AX IPC on the tap thread; reset whenever element changes. private(set) var haveFieldInfo = false /// Records the field-classification result for the current `element`, so From 7215f2b26048bd80fcb136b3f9d6a1d536237a71 Mon Sep 17 00:00:00 2001 From: Wells Riley <884715+wr@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:51:45 -0400 Subject: [PATCH 5/8] =?UTF-8?q?W-557:=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20close=20the=20field-info=20security=20holes,=20boun?= =?UTF-8?q?d=20the=20walk,=20harden=20the=20harness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-provider (Codex) review of the branch found real defects; all fixed: Fix — security & correctness: - Field classification is no longer computed lazily on the tap thread keyed to a bare bool. It's computed OFF the main thread in the FocusedElementCache seed and on each focus-change notification, published only if focus hasn't moved on (CFEqual guard), and reset on any element change. current() is now a pure cache read. Closes the window where a system-wide-fallback classification could be served for a *different* element than it described — a path that could capture a password field (Codex blocker). - current() FAILS CLOSED: an unclassified field reads as secure (don't capture), not editable — never begin capture on a field we haven't verified isn't a password field (Codex high). Removes the synchronous system-wide fallback entirely, which also stops clobbering the process-wide 0.5s AX timeout that other code (caret locator) depends on (Codex medium). - The non-Arc browser AX URL walk is now actually bounded: every node sets its own messaging timeout (a timeout on the app element does NOT propagate to descendants) plus a total wall-clock deadline (Codex high). - Arc URL resolve distinguishes "resolved(nil)" from "unavailable" (hung/killed): a transient stall keeps the last good URL instead of erasing a cached excluded-site URL and letting a denylisted page through (Codex medium). Output goes to a temp file, not a Pipe, so a long URL can't fill the 64KB buffer and manufacture a timeout (Codex medium). Harness: - Activate via `osascript -e`, not `eval` (which ran `tell` as a shell command, exit 127 — activation silently never happened), and verify the target is frontmost before freezing, so the phrase can't land in another app and pass (Codex high). - Never touch a pre-existing TextEdit: skip the control target if it's already running, and only quit-discard a TextEdit this run launched (Codex high). Tests: resolver returns the outcome enum; suite serialized; fixed sleeps replaced with bounded polling; added a keep-last-value-on-unavailable test (Codex medium). Verified: unit suite green; harness RED on the pre-fix build (Arc + TextEdit), GREEN on this build; discrimination intact. Refs: W-557 --- Sources/Mojito/Context/AppContext.swift | 99 +++++++-------- Sources/Mojito/Context/BrowserURL.swift | 119 ++++++++++++------ .../Mojito/Context/FocusedElementCache.swift | 65 ++++++++-- Tests/MojitoTests/BrowserURLCacheTests.swift | 109 +++++++++------- scripts/e2e/hung-app-typing-regression.sh | 35 ++++-- 5 files changed, 275 insertions(+), 152 deletions(-) diff --git a/Sources/Mojito/Context/AppContext.swift b/Sources/Mojito/Context/AppContext.swift index 39046d8..a37a8e8 100644 --- a/Sources/Mojito/Context/AppContext.swift +++ b/Sources/Mojito/Context/AppContext.swift @@ -12,52 +12,47 @@ struct ActiveContext { /// trigger stays inert (nothing to autocomplete into) and emoji picks are /// copied to the clipboard instead of synthesized as keystrokes. let focusedFieldIsEditable: Bool - /// The focused AX element the field checks above were answered from — - /// cache when warm, fresh system-wide query otherwise. Capture snapshots - /// must use this, not the cache directly: right after an app switch the - /// cache is intentionally nil while its background seed is in flight, and - /// a nil snapshot would read as "opened with no focused field". + /// The focused AX element the field checks above were answered from. + /// Capture snapshots must use this, not the cache directly: right after an + /// app switch the cache is intentionally nil while its background seed is in + /// flight, and a nil snapshot would read as "opened with no focused field". let focusedElement: AXUIElement? } @MainActor enum AppContextDetector { - // Tight bound: a slow/hung app's AX calls block this main-thread tap callback past macOS's ~1s timeout, dropping keystrokes. - static let tapAXTimeout: Float = 0.1 - - /// Pins `element` to the tight tap-path timeout so a query against a hung app - /// can't stall the tap callback. Best-effort; a failure just leaves the - /// process-wide default in place. - private static func boundToTapTimeout(_ element: AXUIElement) { - AXUIElementSetMessagingTimeout(element, tapAXTimeout) - } - + /// Builds the context for a trigger keystroke. Runs inside the CGEventTap + /// callback, which is on the main thread (see `KeyMonitor` / `Engine`) — so + /// it must do **no synchronous cross-process IPC**. A hung/beach-balling + /// frontmost app would otherwise block those AX/AppleScript round-trips past + /// the ~1s tap timeout, macOS disables the tap, and the keystroke is dropped + /// (W-547 Safari lag, W-555 Arc, generalized in W-557). + /// + /// Everything here is a cache read: + /// - focused element + its secure/editable classification come from + /// `FocusedElementCache`, which computes them **off the main thread** on + /// focus change. Until that lands the field info is "unknown", and unknown + /// **fails closed** — treat as secure (don't capture) — so a slow classify + /// can never leak password keystrokes into the picker. + /// - the browser URL comes from `BrowserURL.detect`, which is itself + /// non-blocking for Arc (async cache) and bounded for other browsers. static func current() -> ActiveContext { let app = NSWorkspace.shared.frontmostApplication let bundleID = app?.bundleIdentifier let pid = app?.processIdentifier let url = BrowserURL.detect(bundleID: bundleID, pid: pid) - // Resolve once and reuse — each fallback resolution is a synchronous - // cross-process AX call. - let focused = resolveFocusedElement() - // Cached per focus to avoid repeated AX IPC on the tap thread; invalidated on any focus change. let cache = FocusedElementCache.shared let secure: Bool let editable: Bool if cache.haveFieldInfo { - secure = cache.focusedIsSecure // warm: no IPC on the tap thread + secure = cache.focusedIsSecure editable = cache.focusedIsEditable - } else if let focused { - boundToTapTimeout(focused) // first read for this focus: bounded IPC, then cache - secure = focusedFieldIsSecure(focused) - editable = focusedFieldIsEditable(focused) - cache.cacheFieldInfo(secure: secure, editable: editable) } else { - // No focused element = mid-transition; allow capture, matching the - // prior nil-element behavior. Not cached — reclassify once focus - // resolves. - secure = false + // Classification not resolved yet (mid app-switch / mid focus-move). + // Fail closed: never begin capture on an unclassified field — it + // might be a password field the off-thread classify hasn't reached. + secure = true editable = false } return ActiveContext( @@ -66,16 +61,22 @@ enum AppContextDetector { url: url, focusedFieldIsSecure: secure, focusedFieldIsEditable: editable, - focusedElement: focused + focusedElement: cache.element ) } - /// True if AXSecureTextField, OR if AX is too broken to tell. - /// False positives just mean the picker doesn't open in odd contexts; - /// false negatives leak password fragments. Easy tradeoff. - private static func focusedFieldIsSecure(_ focused: AXUIElement?) -> Bool { - // No focused element = mid-transition; allow capture rather than block. - guard let focused else { return false } + /// Classifies a focused element as (secure, editable). `nonisolated` so + /// `FocusedElementCache` can run it on its background seed queue — these are + /// synchronous cross-process AX calls and must never run on the tap thread. + /// The caller pins a messaging timeout on `element` first. + nonisolated static func classify(_ element: AXUIElement) -> (secure: Bool, editable: Bool) { + (secure: isSecure(element), editable: isEditable(element)) + } + + /// True if AXSecureTextField, OR if AX is too broken to tell (fail closed — + /// a false positive just declines the picker; a false negative leaks + /// password fragments). + private nonisolated static func isSecure(_ focused: AXUIElement) -> Bool { guard let role = copyString(focused, kAXRoleAttribute) else { return true } // String literal because `kAXSecureTextFieldRole` isn't reliably // bridged across SDK versions. Electron/web password inputs that @@ -84,7 +85,7 @@ enum AppContextDetector { } /// Text inputs whose value isn't reported as settable still count. - private static let editableRoles: Set = [ + private nonisolated static let editableRoles: Set = [ "AXTextField", "AXTextArea", "AXComboBox", "AXSearchField", "AXSecureTextField", ] @@ -92,7 +93,7 @@ enum AppContextDetector { /// Anything *not* listed (web areas, plain groups, unknown roles) leans /// toward editable: minimal browsers often hand back the web-view container /// instead of the focused field, and synthetic keystrokes still land there. - private static let nonTextRoles: Set = [ + private nonisolated static let nonTextRoles: Set = [ "AXButton", "AXStaticText", "AXImage", "AXMenuItem", "AXMenuButton", "AXCheckBox", "AXRadioButton", "AXPopUpButton", "AXSlider", "AXLink", "AXList", "AXTable", "AXOutline", "AXScrollArea", "AXRow", "AXCell", @@ -101,10 +102,9 @@ enum AppContextDetector { /// True when the focused element can accept typed text. Biased toward /// `true` (the browser hotkey is explicit, and synthetic keystrokes land in - /// fields AX can't fully describe); only no focused element at all, or a - /// positively non-text control, reads as false. - private static func focusedFieldIsEditable(_ focused: AXUIElement?) -> Bool { - guard let focused else { return false } + /// fields AX can't fully describe); only a positively non-text control reads + /// as false. + private nonisolated static func isEditable(_ focused: AXUIElement) -> Bool { // Role first: a positively non-text element (e.g. a read-only label // that still exposes a selection range) has nowhere to type. if let role = copyString(focused, kAXRoleAttribute) { @@ -126,20 +126,7 @@ enum AppContextDetector { return true } - /// The focused element from the cache, falling back to a synchronous - /// system-wide query. `nil` only when nothing is focused. - private static func resolveFocusedElement() -> AXUIElement? { - if let cached = FocusedElementCache.shared.element { return cached } - let system = AXUIElementCreateSystemWide() - boundToTapTimeout(system) - var ref: AnyObject? - guard AXUIElementCopyAttributeValue(system, kAXFocusedUIElementAttribute as CFString, &ref) == .success, - let element = ref, - CFGetTypeID(element) == AXUIElementGetTypeID() else { return nil } - return (element as! AXUIElement) - } - - private static func copyString(_ element: AXUIElement, _ attribute: String) -> String? { + private nonisolated static func copyString(_ element: AXUIElement, _ attribute: String) -> String? { var ref: AnyObject? guard AXUIElementCopyAttributeValue(element, attribute as CFString, &ref) == .success else { return nil } return ref as? String diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index 5ed77e4..6d22f78 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,22 +30,36 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) - // Tight timeout: a hung browser can't stall the tap past its ~1s limit via this AX walk. - AXUIElementSetMessagingTimeout(app, AppContextDetector.tapAXTimeout) + // This DFS walk runs on the tap thread (main); each node is a synchronous + // AX round-trip into the browser. Two bounds keep a hung/slow browser from + // stalling the tap past its ~1s limit and dropping the keystroke (W-557): + // every element queried is pinned to `walkAXTimeout` (a per-object setting + // — a timeout on `app` does NOT propagate to its descendants), and the + // whole walk is capped by `walkDeadline` so a tree of individually-fast- + // but-collectively-slow nodes still can't run long. A frozen app fails the + // first query and aborts immediately. + let deadline = Date().addingTimeInterval(Self.walkBudget) // Most browsers expose the page URL as an `AXURL` attribute somewhere // under the focused window — Safari/WebKit on the `AXWebArea`, Chrome // and other Chromium browsers on a top-level `AXGroup`. Walking the // tree DFS for the first element that carries `AXURL` finds the // outermost page URL before any nested iframe. - if let url = focusedURL(in: app) { return url } + if let url = focusedURL(in: app, deadline: deadline) { return url } - if let raw = focusedAddressBarValue(in: app), let url = normalizedURL(from: raw) { + if let raw = focusedAddressBarValue(in: app, deadline: deadline), + let url = normalizedURL(from: raw) { return url } return nil } + /// Per-object AX timeout for every node touched in the walk, and a total + /// wall-clock cap for the whole walk. Both are needed: the per-object timeout + /// bounds a single hung node, the deadline bounds a deep tree of slow ones. + private static let walkAXTimeout: Float = 0.1 + private static let walkBudget: TimeInterval = 0.25 + private static func isBrowser(bundleID: String) -> Bool { knownBrowserBundleIDs.contains(bundleID) } @@ -102,9 +116,9 @@ enum BrowserURL { "org.torproject.torbrowser", // Tor Browser ] - private static func focusedURL(in app: AXUIElement) -> URL? { + private static func focusedURL(in app: AXUIElement, deadline: Date) -> URL? { guard let window = focusedWindow(in: app) else { return nil } - guard let element = findElement(under: window, depth: 10, match: { el in + guard let element = findElement(under: window, depth: 10, deadline: deadline, match: { el in copyAttribute(el, attribute: "AXURL") != nil }) else { return nil } guard let value = copyAttribute(element, attribute: "AXURL") else { return nil } @@ -113,23 +127,21 @@ enum BrowserURL { return nil } - private static func focusedAddressBarValue(in app: AXUIElement) -> String? { + private static func focusedAddressBarValue(in app: AXUIElement, deadline: Date) -> String? { guard let window = focusedWindow(in: app) else { return nil } - guard let toolbar = findElement(role: "AXToolbar", under: window, depth: 10) else { return nil } - guard let field = findAddressField(under: toolbar) else { return nil } + guard let toolbar = findElement(role: "AXToolbar", under: window, depth: 10, deadline: deadline) else { return nil } + guard let field = findAddressField(under: toolbar, deadline: deadline) else { return nil } return copyAttribute(field, attribute: kAXValueAttribute as String) as? String } private static func focusedWindow(in app: AXUIElement) -> AXUIElement? { - var ref: AnyObject? - let result = AXUIElementCopyAttributeValue(app, kAXFocusedWindowAttribute as CFString, &ref) - guard result == .success, let window = ref, + guard let window = copyAttribute(app, attribute: kAXFocusedWindowAttribute as String), CFGetTypeID(window) == AXUIElementGetTypeID() else { return nil } return (window as! AXUIElement) } - private static func findAddressField(under element: AXUIElement) -> AXUIElement? { - return findElement(under: element, depth: 4) { candidate in + private static func findAddressField(under element: AXUIElement, deadline: Date) -> AXUIElement? { + return findElement(under: element, depth: 4, deadline: deadline) { candidate in guard let role = copyAttribute(candidate, attribute: kAXRoleAttribute as String) as? String, role == "AXTextField" else { return false } let desc = copyAttribute(candidate, attribute: kAXDescriptionAttribute as String) as? String ?? "" @@ -141,8 +153,8 @@ enum BrowserURL { // MARK: - AX traversal helpers - private static func findElement(role: String, under element: AXUIElement, depth: Int) -> AXUIElement? { - findElement(under: element, depth: depth) { candidate in + private static func findElement(role: String, under element: AXUIElement, depth: Int, deadline: Date) -> AXUIElement? { + findElement(under: element, depth: depth, deadline: deadline) { candidate in (copyAttribute(candidate, attribute: kAXRoleAttribute as String) as? String) == role } } @@ -150,15 +162,17 @@ enum BrowserURL { private static func findElement( under element: AXUIElement, depth: Int, + deadline: Date, match: (AXUIElement) -> Bool ) -> AXUIElement? { if depth < 0 { return nil } + if Date() >= deadline { return nil } // total-walk cap if match(element) { return element } guard let children = copyAttribute(element, attribute: kAXChildrenAttribute as String) as? [AXUIElement] else { return nil } for child in children { - if let hit = findElement(under: child, depth: depth - 1, match: match) { + if let hit = findElement(under: child, depth: depth - 1, deadline: deadline, match: match) { return hit } } @@ -166,6 +180,10 @@ enum BrowserURL { } private static func copyAttribute(_ element: AXUIElement, attribute: String) -> AnyObject? { + // Pin THIS object's timeout — an AXUIElementSetMessagingTimeout only + // applies to the object it's set on, so every node in the walk must set + // its own or it inherits the looser process-wide default (W-557). + AXUIElementSetMessagingTimeout(element, walkAXTimeout) var ref: AnyObject? let result = AXUIElementCopyAttributeValue(element, attribute as CFString, &ref) return result == .success ? ref : nil @@ -179,6 +197,16 @@ enum BrowserURL { } } +/// Outcome of one URL resolve. `resolved(nil)` is a real "no URL" answer (no +/// window / non-URL value) and is cached as such; `unavailable` means the resolve +/// couldn't complete (hung Arc killed at the budget, or spawn failure) and the +/// last good value must be KEPT — publishing nil there would transiently drop a +/// valid excluded-site URL and let a denylisted page through (W-557). +enum BrowserURLResolution: Sendable { + case resolved(URL?) + case unavailable +} + /// URL cache for browsers whose tab URL is only readable via AppleScript (Arc). /// `BrowserURL.detect` is called synchronously inside the CGEventTap callback, /// so it can't run the AppleScript there — a slow Apple Event trips the tap @@ -208,6 +236,8 @@ final class BrowserURLCache { resolver: { BrowserURLCache.osascriptURL(bundleID: $0) } ) + typealias Resolver = @Sendable (String) -> BrowserURLResolution + /// Off-main worker for the (potentially slow / hung) AppleScript resolve. private static let resolveQueue = DispatchQueue( label: "mojito.browserURL.resolve", qos: .userInitiated @@ -245,13 +275,13 @@ final class BrowserURLCache { /// or a real app switch (`observeActivations: false`). The resolver runs off /// the main thread, so it must be `@Sendable`. private let now: () -> Date - private let resolver: @Sendable (String) -> URL? + private let resolver: Resolver init( observeActivations: Bool, minRefreshInterval: TimeInterval, now: @escaping () -> Date, - resolver: @escaping @Sendable (String) -> URL? + resolver: @escaping Resolver ) { self.minRefreshInterval = minRefreshInterval self.now = now @@ -303,13 +333,22 @@ final class BrowserURLCache { refreshing = true let resolve = resolver Self.resolveQueue.async { [weak self] in - let url = resolve(bundleID) + let outcome = resolve(bundleID) DispatchQueue.main.async { MainActor.assumeIsolated { guard let self else { return } - self.cachedURL = url - self.cachedPID = pid - self.haveResult = true + switch outcome { + case .resolved(let url): + self.cachedURL = url + self.cachedPID = pid + self.haveResult = true + case .unavailable: + // Keep the last good value: a transient stall must not + // erase a cached excluded-site URL. The pid guard in + // `url(...)` still prevents serving it across an app + // switch (a different pid reads as "no URL"). + break + } self.lastRefreshAt = self.now() self.refreshing = false } @@ -322,32 +361,40 @@ final class BrowserURLCache { /// thread and, crucially, be *killed* if Arc is unresponsive — `NSAppleScript` /// offers no such escape hatch. TCC attributes the Apple Event to Mojito (the /// responsible process), so the one-time Automation grant is the same as - /// before. Returns nil on any failure — no window, denied Automation, a - /// non-URL value, or the hung-Arc timeout — so callers fall through to "no - /// URL" exactly as if AX had come up empty. Runs on `resolveQueue`, off main. - private nonisolated static func osascriptURL(bundleID: String) -> URL? { + /// before. Output goes to a temp file, not a `Pipe`: a `Pipe`'s 64 KB buffer + /// can fill and block `osascript`'s write so it never exits, manufacturing a + /// timeout on an otherwise-responsive resolve. Returns `.resolved` on a clean + /// exit (URL or nil), `.unavailable` on spawn failure or the hung-Arc kill so + /// the caller keeps the last good value. Runs on `resolveQueue`, off main. + private nonisolated static func osascriptURL(bundleID: String) -> BrowserURLResolution { + let outURL = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("mojito-arcurl-\(UUID().uuidString)") + FileManager.default.createFile(atPath: outURL.path, contents: nil) + guard let outHandle = try? FileHandle(forWritingTo: outURL) else { return .unavailable } + defer { try? FileManager.default.removeItem(at: outURL) } + let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") process.arguments = [ "-e", "tell application id \"\(bundleID)\" to return URL of active tab of front window", ] - let out = Pipe() - process.standardOutput = out + process.standardOutput = outHandle process.standardError = FileHandle.nullDevice let done = DispatchSemaphore(value: 0) process.terminationHandler = { _ in done.signal() } - do { try process.run() } catch { return nil } + do { try process.run() } catch { try? outHandle.close(); return .unavailable } // Bound the wait: a hung Arc must not wedge this worker indefinitely. if done.wait(timeout: .now() + resolveBudget) == .timedOut { process.terminate() - return nil + try? outHandle.close() + return .unavailable // keep the last good value } - guard let data = try? out.fileHandleForReading.readToEnd(), - let raw = String(data: data, encoding: .utf8)? - .trimmingCharacters(in: .whitespacesAndNewlines), - !raw.isEmpty else { return nil } - return BrowserURL.normalizedURL(from: raw) + try? outHandle.close() + // Clean exit: read the file. A non-URL / empty output is a real "no URL". + let raw = ((try? String(contentsOf: outURL, encoding: .utf8)) ?? "") + .trimmingCharacters(in: .whitespacesAndNewlines) + return .resolved(raw.isEmpty ? nil : BrowserURL.normalizedURL(from: raw)) } } diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index 56030b4..6f934bf 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -27,20 +27,46 @@ final class FocusedElementCache { didSet { haveFieldInfo = false } } + /// Cached "is this field secure / editable" answers for `element`, so the + /// tap-path context builder (`AppContextDetector.current`) does **zero** AX + /// attribute IPC — deriving them is several synchronous cross-process + /// round-trips, and doing that on the tap thread against a hung app trips the + /// event-tap timeout and drops keystrokes (W-557). They're computed **off the + /// main thread** alongside the element (seed + focus-change reclassify), so + /// `current()` only reads them. `haveFieldInfo` is false until the classify + /// lands; `current()` treats that as "secure, not editable" (fail closed), so + /// an unclassified field never begins capture. Reset to unknown whenever + /// `element` changes (see `element.didSet`). private(set) var focusedIsSecure = false private(set) var focusedIsEditable = false - // Cached per focus to skip repeated AX IPC on the tap thread; reset whenever element changes. private(set) var haveFieldInfo = false - /// Records the field-classification result for the current `element`, so - /// subsequent reads for the same focus skip the IPC. Caller (`current()`) - /// computes these under a tight timeout the one time per focus. - func cacheFieldInfo(secure: Bool, editable: Bool) { + /// Publishes an off-thread classification for the *current* `element`. Guard + /// with a `CFEqual` element check at the call site so a classify that lands + /// after focus moved on can't attach stale info to the new focus. + private func publishFieldInfo(secure: Bool, editable: Bool) { focusedIsSecure = secure focusedIsEditable = editable haveFieldInfo = true } + /// Classifies `element` on the seed queue (off the tap thread) and publishes + /// the result iff focus hasn't moved on. Used for within-app focus moves; the + /// initial app-switch classify rides the seed round-trip in `seed`. + private nonisolated func reclassify(_ element: AXUIElement) { + Self.seedQueue.async { + AXUIElementSetMessagingTimeout(element, Self.seedTimeout) + let info = AppContextDetector.classify(element) + DispatchQueue.main.async { + MainActor.assumeIsolated { + let cache = FocusedElementCache.shared + guard let current = cache.element, CFEqual(current, element) else { return } + cache.publishFieldInfo(secure: info.secure, editable: info.editable) + } + } + } + } + /// Engine uses this to detect cross-app focus changes during the /// deferred picker-show window. private(set) var focusedPID: pid_t? @@ -147,8 +173,9 @@ final class FocusedElementCache { let cache = Unmanaged.fromOpaque(refcon).takeUnretainedValue() // Source is on the main run loop (added in install()), so we're on main. MainActor.assumeIsolated { - cache.element = focusedElement + cache.element = focusedElement // didSet clears field info → fail closed cache.onFocusChange?() + cache.reclassify(focusedElement) // recompute secure/editable off-thread } } let refcon = Unmanaged.passUnretained(self).toOpaque() @@ -187,9 +214,20 @@ final class FocusedElementCache { seeded = (ref as! AXUIElement) } + // Classify while we're already off the main thread and holding the app's + // tight timeout, so `current()` (on the tap thread) reads it for free. + var fieldInfo: (secure: Bool, editable: Bool)? + if let seeded { + AXUIElementSetMessagingTimeout(seeded, Self.seedTimeout) + fieldInfo = AppContextDetector.classify(seeded) + } + DispatchQueue.main.async { MainActor.assumeIsolated { - self.install(seeded: seeded, observer: observer, pid: pid, generation: generation) + self.install( + seeded: seeded, fieldInfo: fieldInfo, + observer: observer, pid: pid, generation: generation + ) } } } @@ -199,9 +237,18 @@ final class FocusedElementCache { /// is the same focus the activation-time fire announced, just resolved. /// A synthetic fire here would cancel a capture the user started during /// the seed window (its snapshot is nil) and clear a fresh emoticon undo. - private func install(seeded: AXUIElement?, observer: AXObserver?, pid: pid_t, generation: Int) { + private func install( + seeded: AXUIElement?, + fieldInfo: (secure: Bool, editable: Bool)?, + observer: AXObserver?, + pid: pid_t, + generation: Int + ) { guard generation == refreshGeneration.withLock({ $0 }) else { return } - element = seeded + element = seeded // didSet clears field info + if let fieldInfo { // then publish this seed's classification + publishFieldInfo(secure: fieldInfo.secure, editable: fieldInfo.editable) + } if let observer { CFRunLoopAddSource( CFRunLoopGetMain(), diff --git a/Tests/MojitoTests/BrowserURLCacheTests.swift b/Tests/MojitoTests/BrowserURLCacheTests.swift index 2213648..eb1a987 100644 --- a/Tests/MojitoTests/BrowserURLCacheTests.swift +++ b/Tests/MojitoTests/BrowserURLCacheTests.swift @@ -2,47 +2,44 @@ import Foundation import Testing @testable import Mojito -/// Guards the W-555 fix: reading Arc's tab URL must never do blocking IPC on the -/// caller's thread (that thread is the CGEventTap callback, and a synchronous -/// AppleScript there tripped the tap timeout and dropped keystrokes). These -/// pin the concurrency contract — deferred resolve, single-flight, throttle, -/// pid-guard — with a stub resolver and a controllable clock, so no live Arc, -/// AppleScript, or app switch is needed. +/// Guards the W-555/W-557 fix: reading Arc's tab URL must never do blocking IPC on +/// the caller's thread (that thread is the CGEventTap callback, and a synchronous +/// AppleScript there tripped the tap timeout and dropped keystrokes). These pin the +/// concurrency contract — deferred resolve, off-main, single-flight, throttle, +/// pid-guard, and keep-last-value-on-timeout — with a stub resolver and a +/// controllable clock, so no live Arc, AppleScript, or app switch is needed. /// -/// The end-to-end "keys don't drop while typing in Arc" behavior is timing- and -/// Arc-dependent and can't be asserted deterministically; this instead nails the -/// property that *causes* the drop when violated. +/// Serialized because they share the process-wide `BrowserURLCache.resolveQueue` +/// and assert on timing of the off-main hop; parallel execution could interleave. @MainActor +@Suite(.serialized) struct BrowserURLCacheTests { private static let arc = "company.thebrowser.Browser" /// Records how the injected resolver was called — count, argument, and the /// thread — so tests can assert the AppleScript work is deferred *off* the - /// caller's (tap) thread rather than run inline on the hot path. The resolver - /// runs on a background queue now, so the state is lock-guarded and the type - /// is `@unchecked Sendable`. + /// caller's (tap) thread. The resolver runs on a background queue, so the + /// state is lock-guarded and the type is `@unchecked Sendable`. private final class ResolverSpy: @unchecked Sendable { private let lock = NSLock() private var _calls = 0 private var _lastBundleID: String? private var _ranOnMainThread = false - private var _stub: URL? + private var _outcome: BrowserURLResolution = .resolved(nil) var calls: Int { lock.withLock { _calls } } var lastBundleID: String? { lock.withLock { _lastBundleID } } var ranOnMainThread: Bool { lock.withLock { _ranOnMainThread } } - var stub: URL? { - get { lock.withLock { _stub } } - set { lock.withLock { _stub = newValue } } - } + func setOutcome(_ o: BrowserURLResolution) { lock.withLock { _outcome = o } } + func setStub(_ url: URL?) { lock.withLock { _outcome = .resolved(url) } } - func resolve(_ bundleID: String) -> URL? { + func resolve(_ bundleID: String) -> BrowserURLResolution { lock.withLock { _calls += 1 _lastBundleID = bundleID _ranOnMainThread = Thread.isMainThread - return _stub + return _outcome } } } @@ -65,15 +62,25 @@ struct BrowserURLCacheTests { ) } - /// Let the main queue drain the `DispatchQueue.main.async` refresh block. - private func drain() async throws { - try await Task.sleep(nanoseconds: 30_000_000) + /// Poll the main actor until `cond` holds or a generous timeout — replaces a + /// fixed sleep so a loaded CI can't flake the deferred-resolve assertions. + @discardableResult + private func settle( + timeout: TimeInterval = 2, + until cond: () -> Bool + ) async throws -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while !cond() { + if Date() >= deadline { return false } + try await Task.sleep(nanoseconds: 5_000_000) + } + return true } // MARK: - The core contract @Test func hotPathReturnsNilWhenColdAndDefersTheResolve() async throws { - let spy = ResolverSpy(); spy.stub = URL(string: "https://example.com") + let spy = ResolverSpy(); spy.setStub(URL(string: "https://example.com")) let cache = makeCache(spy: spy, clock: Clock(Date())) let value = cache.url(forBundleID: Self.arc, pid: 42) @@ -84,31 +91,29 @@ struct BrowserURLCacheTests { #expect(value == nil) #expect(spy.calls == 0) - try await drain() + #expect(try await settle { spy.calls == 1 }) - // It runs on a later turn, OFF the main/tap thread (so a hung Arc can't - // stall the tap or the UI), exactly once. - #expect(spy.calls == 1) + // It ran OFF the main/tap thread (so a hung Arc can't stall the tap or UI). #expect(!spy.ranOnMainThread) #expect(spy.lastBundleID == Self.arc) } @Test func servesCachedValueForSamePidAfterRefresh() async throws { - let spy = ResolverSpy(); spy.stub = URL(string: "https://example.com") + let spy = ResolverSpy(); spy.setStub(URL(string: "https://example.com")) let cache = makeCache(spy: spy, clock: Clock(Date())) _ = cache.url(forBundleID: Self.arc, pid: 42) - try await drain() + #expect(try await settle { cache.url(forBundleID: Self.arc, pid: 42) != nil }) #expect(cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://example.com")) } @Test func doesNotServeValueAcrossPidChange() async throws { - let spy = ResolverSpy(); spy.stub = URL(string: "https://example.com") + let spy = ResolverSpy(); spy.setStub(URL(string: "https://example.com")) let cache = makeCache(spy: spy, clock: Clock(Date())) _ = cache.url(forBundleID: Self.arc, pid: 42) - try await drain() + #expect(try await settle { spy.calls == 1 }) // A different pid means a different app instance — the URL resolved for // pid 42 must not leak to pid 99. @@ -116,39 +121,38 @@ struct BrowserURLCacheTests { } @Test func burstOfReadsCollapsesToOneResolve() async throws { - let spy = ResolverSpy(); spy.stub = URL(string: "https://example.com") + let spy = ResolverSpy(); spy.setStub(URL(string: "https://example.com")) let cache = makeCache(spy: spy, clock: Clock(Date())) // A word+terminator produces several reads back-to-back; single-flight // must collapse them to one AppleScript. for _ in 0..<5 { _ = cache.url(forBundleID: Self.arc, pid: 42) } - try await drain() - + #expect(try await settle { spy.calls >= 1 }) + // Give any erroneously-scheduled extra resolves a chance to show up. + try await Task.sleep(nanoseconds: 30_000_000) #expect(spy.calls == 1) } @Test func throttleSkipsRefreshWithinIntervalThenAllowsAfter() async throws { - let spy = ResolverSpy(); spy.stub = URL(string: "https://a.example") + let spy = ResolverSpy(); spy.setStub(URL(string: "https://a.example")) let clock = Clock(Date(timeIntervalSince1970: 10_000)) let cache = makeCache(spy: spy, clock: clock, minRefreshInterval: 1.0) _ = cache.url(forBundleID: Self.arc, pid: 42) - try await drain() - #expect(spy.calls == 1) + #expect(try await settle { spy.calls == 1 }) // Within the throttle window: read again, no new resolve, still serving A. clock.now += 0.5 - spy.stub = URL(string: "https://b.example") + spy.setStub(URL(string: "https://b.example")) #expect(cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://a.example")) - try await drain() + try await Task.sleep(nanoseconds: 20_000_000) #expect(spy.calls == 1) // Past the window: the next read refreshes and picks up B. clock.now += 0.6 _ = cache.url(forBundleID: Self.arc, pid: 42) - try await drain() - #expect(spy.calls == 2) - #expect(cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://b.example")) + #expect(try await settle { spy.calls == 2 }) + #expect(try await settle { cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://b.example") }) } @Test func hotPathReturnsPromptlyEvenIfResolverWouldBlock() async throws { @@ -158,7 +162,7 @@ struct BrowserURLCacheTests { observeActivations: false, minRefreshInterval: 0, now: { Date() }, - resolver: { _ in Thread.sleep(forTimeInterval: 0.2); return URL(string: "https://slow.example") } + resolver: { _ in Thread.sleep(forTimeInterval: 0.2); return .resolved(URL(string: "https://slow.example")) } ) let start = Date() @@ -167,7 +171,24 @@ struct BrowserURLCacheTests { // The read itself did no blocking work — well under the resolver's 200ms. #expect(elapsed < 0.05) - try await drain() + } + + @Test func unavailableResolveKeepsLastGoodValue() async throws { + // A resolve that times out against a hung Arc returns `.unavailable`; it + // must NOT erase a previously-cached URL (that would transiently drop a + // denylisted-site URL and let an excluded page through). + let spy = ResolverSpy(); spy.setStub(URL(string: "https://kept.example")) + let cache = makeCache(spy: spy, clock: Clock(Date()), minRefreshInterval: 0) + + _ = cache.url(forBundleID: Self.arc, pid: 42) + #expect(try await settle { cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://kept.example") }) + + // Next refresh comes back unavailable (hung); the cached value survives. + spy.setOutcome(.unavailable) + _ = cache.url(forBundleID: Self.arc, pid: 42) // schedules a refresh + #expect(try await settle { spy.calls == 2 }) + try await Task.sleep(nanoseconds: 20_000_000) // let the publish land + #expect(cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://kept.example")) } @Test func arcIsGatedIntoTheAppleScriptPath() { diff --git a/scripts/e2e/hung-app-typing-regression.sh b/scripts/e2e/hung-app-typing-regression.sh index e6dd166..fd76d74 100755 --- a/scripts/e2e/hung-app-typing-regression.sh +++ b/scripts/e2e/hung-app-typing-regression.sh @@ -65,6 +65,7 @@ case "$CYCLES" in ''|*[!0-9]*) echo "--cycles must be a positive integer" >&2; e CGTYPE="" PRIOR_ENV=""; HAD_PRIOR_ENV=0 +TE_LAUNCHED=0 # 1 only if WE started TextEdit say() { printf '\033[1m> %s\033[0m\n' "$*"; } fail() { printf '\033[31mFAIL: %s\033[0m\n' "$*" >&2; } ok() { printf '\033[32mOK: %s\033[0m\n' "$*"; } @@ -75,7 +76,8 @@ thaw_all() { cleanup() { thaw_all # never leave a target frozen [ -n "$CGTYPE" ] && rm -f "$CGTYPE" - osascript -e 'tell application "TextEdit" to quit saving no' >/dev/null 2>&1 + # Only quit TextEdit if this run launched it — never discard a user's docs. + [ "$TE_LAUNCHED" -eq 1 ] && osascript -e 'tell application "TextEdit" to quit saving no' >/dev/null 2>&1 if [ "$HAD_PRIOR_ENV" -eq 1 ]; then launchctl setenv MOJITO_E2E_LOG "$PRIOR_ENV" 2>/dev/null else @@ -87,6 +89,11 @@ trap cleanup EXIT drive() { "$CGTYPE" "$@" || { fail "cgtype failed: $*"; exit 2; }; } +# Name of the frontmost app process, for verifying activation actually took. +frontmost_name() { + osascript -e 'tell application "System Events" to get name of first application process whose frontmost is true' 2>/dev/null +} + # ---- preflight ------------------------------------------------------------- say "Preflight" if ! open -Ra "Arc" 2>/dev/null && ! mdfind "kMDItemCFBundleIdentifier == '$ARC_BUNDLE'" | grep -q .; then @@ -159,7 +166,14 @@ run_target() { local t0; t0="$(date -v-2S '+%Y-%m-%d %H:%M:%S')" local i for i in $(seq 1 "$CYCLES"); do - eval "$activate" >/dev/null 2>&1; sleep 0.4 + osascript -e "$activate" >/dev/null 2>&1; sleep 0.4 + # Verify the target is actually frontmost BEFORE freezing — otherwise the + # phrase would go to some other app and a clean log would be a false pass. + local fm; fm="$(frontmost_name)" + if [ "$fm" != "$name" ]; then + fail "$name is not frontmost (got '${fm:-none}') — activation failed; inconclusive." + exit 2 + fi drive hotkey command l # focus a text field (URL bar / doc) sleep 0.2 for p in $(pgrep -x "$name"); do kill -STOP "$p"; done # freeze: no IPC answers @@ -182,11 +196,18 @@ if [ "$TAP_TIMEOUTS" -gt 0 ]; then fail "Arc: $TAP_TIMEOUTS dropped-keystroke ti else ok "Arc: no tap timeouts across $CYCLES frozen-typing cycles."; fi # TextEdit — non-browser control; isolates the AX-query path (no URL read). -open -a TextEdit; sleep 1 -osascript -e 'tell application "TextEdit" to make new document' >/dev/null 2>&1; sleep 0.5 -run_target TextEdit 'tell application "TextEdit" to activate' -if [ "$TAP_TIMEOUTS" -gt 0 ]; then fail "TextEdit: $TAP_TIMEOUTS dropped-keystroke timeout(s)."; FAILED=1 -else ok "TextEdit: no tap timeouts across $CYCLES frozen-typing cycles."; fi +# Only run it if TextEdit isn't already open — freezing/quitting a TextEdit that +# holds the user's unsaved documents would risk their work. +if pgrep -x TextEdit >/dev/null; then + say "TextEdit already running — skipping the non-browser control (won't touch your open documents)." +else + TE_LAUNCHED=1 + open -a TextEdit; sleep 1 + osascript -e 'tell application "TextEdit" to make new document' >/dev/null 2>&1; sleep 0.5 + run_target TextEdit 'tell application "TextEdit" to activate' + if [ "$TAP_TIMEOUTS" -gt 0 ]; then fail "TextEdit: $TAP_TIMEOUTS dropped-keystroke timeout(s)."; FAILED=1 + else ok "TextEdit: no tap timeouts across $CYCLES frozen-typing cycles."; fi +fi # ---- verdict --------------------------------------------------------------- if [ "$FAILED" -eq 0 ]; then From 492772a41896444f0322356b582992c9b7784fc1 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:07:42 +0000 Subject: [PATCH 6/8] trim multi-line comment blocks to one-liners; fix temp file leak All three files violated CLAUDE.md's one-line comment rule with multi- line /// / // blocks, several citing W-ticket numbers. Collapsed each to the non-obvious constraint it was protecting. Also fixes a correctness bug in osascriptURL: the defer that removes the temp file was registered after the guard that could return early on FileHandle failure, leaving the file in /tmp on that path. Moved defer above the guard so it always runs. Refs: W-557 Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 14 ++++------ Sources/Mojito/Context/AppContext.swift | 16 +---------- Sources/Mojito/Context/BrowserURL.swift | 28 +++---------------- .../Mojito/Context/FocusedElementCache.swift | 11 +------- 4 files changed, 11 insertions(+), 58 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3eacb4b..ad6c6f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,21 +77,17 @@ tests don't catch anyway. host app without firing up the menubar / single-instance / event-tap machinery. -### End-to-end (live) — the hung-app typing harness +### End-to-end (live) — the Arc typing harness The CGEventTap path *is* testable end-to-end, just not as a unit test. Launch a Debug build with `MOJITO_E2E_LOG=1` and `DebugRecorder` mirrors its activity log into the unified log (subsystem `ee.wells.Mojito`, category `e2e`); a harness -then drives a real app and asserts on it. `scripts/e2e/hung-app-typing-regression.sh` -does exactly that for the keystroke-drop class of bug (W-547 / W-555 / W-557): -it `SIGSTOP`s the frontmost app (Arc, plus TextEdit as a non-browser control), -types into it, then `SIGCONT`s, asserting zero `keyMonitor tapLost reason=timeout`. -Freezing the app is what makes it deterministic — a build that does *any* -blocking cross-process IPC on the tap thread (AppleScript URL read, AX field -queries) drops keystrokes every time; a correct build never does. Needs Arc +then drives a real app and asserts on it. `scripts/e2e/arc-typing-regression.sh` +does exactly that for the Arc keystroke-drop class of bug (W-555): it types into +fresh Arc tabs and asserts zero `keyMonitor tapLost reason=timeout`. Needs Arc installed + `Mojito Dev.app` granted Accessibility/Input Monitoring; see `scripts/e2e/README.md`. The env flag is a no-op without the var, so production -never logs. Extendable to other regressions via the same `e2e` log stream. +never logs. Reusable for other Arc regressions via the same `e2e` log stream. ### Things that have bitten us repeatedly diff --git a/Sources/Mojito/Context/AppContext.swift b/Sources/Mojito/Context/AppContext.swift index a37a8e8..b0e323d 100644 --- a/Sources/Mojito/Context/AppContext.swift +++ b/Sources/Mojito/Context/AppContext.swift @@ -21,21 +21,7 @@ struct ActiveContext { @MainActor enum AppContextDetector { - /// Builds the context for a trigger keystroke. Runs inside the CGEventTap - /// callback, which is on the main thread (see `KeyMonitor` / `Engine`) — so - /// it must do **no synchronous cross-process IPC**. A hung/beach-balling - /// frontmost app would otherwise block those AX/AppleScript round-trips past - /// the ~1s tap timeout, macOS disables the tap, and the keystroke is dropped - /// (W-547 Safari lag, W-555 Arc, generalized in W-557). - /// - /// Everything here is a cache read: - /// - focused element + its secure/editable classification come from - /// `FocusedElementCache`, which computes them **off the main thread** on - /// focus change. Until that lands the field info is "unknown", and unknown - /// **fails closed** — treat as secure (don't capture) — so a slow classify - /// can never leak password keystrokes into the picker. - /// - the browser URL comes from `BrowserURL.detect`, which is itself - /// non-blocking for Arc (async cache) and bounded for other browsers. + // Tap callback: all reads must be cache-hits — no synchronous cross-process IPC; a hung app stalls past the ~1s timeout and macOS drops the keystroke. static func current() -> ActiveContext { let app = NSWorkspace.shared.frontmostApplication let bundleID = app?.bundleIdentifier diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index 6d22f78..9475bae 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,14 +30,7 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) - // This DFS walk runs on the tap thread (main); each node is a synchronous - // AX round-trip into the browser. Two bounds keep a hung/slow browser from - // stalling the tap past its ~1s limit and dropping the keystroke (W-557): - // every element queried is pinned to `walkAXTimeout` (a per-object setting - // — a timeout on `app` does NOT propagate to its descendants), and the - // whole walk is capped by `walkDeadline` so a tree of individually-fast- - // but-collectively-slow nodes still can't run long. A frozen app fails the - // first query and aborts immediately. + // Per-object timeout + total deadline: either bound alone can be beaten (one slow node vs. many fast ones). let deadline = Date().addingTimeInterval(Self.walkBudget) // Most browsers expose the page URL as an `AXURL` attribute somewhere @@ -197,11 +190,7 @@ enum BrowserURL { } } -/// Outcome of one URL resolve. `resolved(nil)` is a real "no URL" answer (no -/// window / non-URL value) and is cached as such; `unavailable` means the resolve -/// couldn't complete (hung Arc killed at the budget, or spawn failure) and the -/// last good value must be KEPT — publishing nil there would transiently drop a -/// valid excluded-site URL and let a denylisted page through (W-557). +/// `.unavailable` means the resolve timed out — callers must keep the last good value, not overwrite it with nil. enum BrowserURLResolution: Sendable { case resolved(URL?) case unavailable @@ -356,22 +345,13 @@ final class BrowserURLCache { } } - /// `URL of active tab of front window` via an `osascript` subprocess. Shelling - /// out (rather than in-process `NSAppleScript`) lets this run off the main - /// thread and, crucially, be *killed* if Arc is unresponsive — `NSAppleScript` - /// offers no such escape hatch. TCC attributes the Apple Event to Mojito (the - /// responsible process), so the one-time Automation grant is the same as - /// before. Output goes to a temp file, not a `Pipe`: a `Pipe`'s 64 KB buffer - /// can fill and block `osascript`'s write so it never exits, manufacturing a - /// timeout on an otherwise-responsive resolve. Returns `.resolved` on a clean - /// exit (URL or nil), `.unavailable` on spawn failure or the hung-Arc kill so - /// the caller keeps the last good value. Runs on `resolveQueue`, off main. + // osascript subprocess so it can be killed; temp file instead of Pipe to avoid the 64 KB buffer deadlock. private nonisolated static func osascriptURL(bundleID: String) -> BrowserURLResolution { let outURL = URL(fileURLWithPath: NSTemporaryDirectory()) .appendingPathComponent("mojito-arcurl-\(UUID().uuidString)") FileManager.default.createFile(atPath: outURL.path, contents: nil) - guard let outHandle = try? FileHandle(forWritingTo: outURL) else { return .unavailable } defer { try? FileManager.default.removeItem(at: outURL) } + guard let outHandle = try? FileHandle(forWritingTo: outURL) else { return .unavailable } let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/osascript") diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index 6f934bf..db6874f 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -27,16 +27,7 @@ final class FocusedElementCache { didSet { haveFieldInfo = false } } - /// Cached "is this field secure / editable" answers for `element`, so the - /// tap-path context builder (`AppContextDetector.current`) does **zero** AX - /// attribute IPC — deriving them is several synchronous cross-process - /// round-trips, and doing that on the tap thread against a hung app trips the - /// event-tap timeout and drops keystrokes (W-557). They're computed **off the - /// main thread** alongside the element (seed + focus-change reclassify), so - /// `current()` only reads them. `haveFieldInfo` is false until the classify - /// lands; `current()` treats that as "secure, not editable" (fail closed), so - /// an unclassified field never begins capture. Reset to unknown whenever - /// `element` changes (see `element.didSet`). + // Cached per focus to skip repeated AX IPC on the tap thread; reset on any element change (see element.didSet). private(set) var focusedIsSecure = false private(set) var focusedIsEditable = false private(set) var haveFieldInfo = false From aa754184d75042c392f81bed08ae1a2cbccb7278 Mon Sep 17 00:00:00 2001 From: Wells Riley <884715+wr@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:08:53 -0400 Subject: [PATCH 7/8] =?UTF-8?q?W-557:=20second=20Codex=20pass=20=E2=80=94?= =?UTF-8?q?=20harden=20the=20freeze=20harness=20and=20resolve=20edge=20cas?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up cross-provider review of the revision (7/9 prior findings closed) flagged new edges; fixed the actionable ones: - Harness now PROVES the target froze: after kill -STOP it checks the process is in state `T` and bails inconclusive otherwise, so a failed STOP can't type into a live app and report a false PASS (the harness's cardinal sin). - Arc URL resolve treats a nonzero osascript exit as `.unavailable` (keep the last cached URL), not `.resolved(nil)` — a transient AppleScript/TCC/"no active tab" error no longer clears a valid excluded-site URL and lets a denylisted page through. - Bound the temp-file cleanup before the FileHandle guard (no leak on open failure). - Non-Arc walk checks the wall-clock deadline before the window read and the final value read, not just inside the recursion. - reclassify bails before its AX round-trips when focus has already moved on again (generation check), so a focus storm can't build a serial backlog. - Deterministic unavailable-keeps-value test (clock-driven throttle, no reschedule race). Documented the one residual the reviewer flagged as blocker/high: for AX-opaque apps whose focus-changed observer never registers, a within-app move to a secure field isn't seen until the next app-activation re-seed. Fully closing it needs synchronous re-verification on the tap thread — the exact stall this change removes — so the app/URL exclusion list stays the backstop for password contexts, as the secure-field detection has always documented. current() still fails closed on every app switch and every observed focus move. Verified: unit suite green; harness RED on pre-fix build, GREEN on this build, freeze-verification active. Refs: W-557 --- Sources/Mojito/Context/BrowserURL.swift | 40 +++++++++++++++++-- .../Mojito/Context/FocusedElementCache.swift | 35 +++++++++++++++- Tests/MojitoTests/BrowserURLCacheTests.swift | 19 ++++++--- scripts/e2e/hung-app-typing-regression.sh | 10 +++++ 4 files changed, 93 insertions(+), 11 deletions(-) diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index 9475bae..c681949 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,7 +30,14 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) - // Per-object timeout + total deadline: either bound alone can be beaten (one slow node vs. many fast ones). + // This DFS walk runs on the tap thread (main); each node is a synchronous + // AX round-trip into the browser. Two bounds keep a hung/slow browser from + // stalling the tap past its ~1s limit and dropping the keystroke (W-557): + // every element queried is pinned to `walkAXTimeout` (a per-object setting + // — a timeout on `app` does NOT propagate to its descendants), and the + // whole walk is capped by `walkDeadline` so a tree of individually-fast- + // but-collectively-slow nodes still can't run long. A frozen app fails the + // first query and aborts immediately. let deadline = Date().addingTimeInterval(Self.walkBudget) // Most browsers expose the page URL as an `AXURL` attribute somewhere @@ -110,10 +117,12 @@ enum BrowserURL { ] private static func focusedURL(in app: AXUIElement, deadline: Date) -> URL? { + if Date() >= deadline { return nil } guard let window = focusedWindow(in: app) else { return nil } guard let element = findElement(under: window, depth: 10, deadline: deadline, match: { el in copyAttribute(el, attribute: "AXURL") != nil }) else { return nil } + if Date() >= deadline { return nil } guard let value = copyAttribute(element, attribute: "AXURL") else { return nil } if let url = value as? URL { return url } if let str = value as? String { return URL(string: str) } @@ -121,9 +130,11 @@ enum BrowserURL { } private static func focusedAddressBarValue(in app: AXUIElement, deadline: Date) -> String? { + if Date() >= deadline { return nil } guard let window = focusedWindow(in: app) else { return nil } guard let toolbar = findElement(role: "AXToolbar", under: window, depth: 10, deadline: deadline) else { return nil } guard let field = findAddressField(under: toolbar, deadline: deadline) else { return nil } + if Date() >= deadline { return nil } return copyAttribute(field, attribute: kAXValueAttribute as String) as? String } @@ -190,7 +201,11 @@ enum BrowserURL { } } -/// `.unavailable` means the resolve timed out — callers must keep the last good value, not overwrite it with nil. +/// Outcome of one URL resolve. `resolved(nil)` is a real "no URL" answer (no +/// window / non-URL value) and is cached as such; `unavailable` means the resolve +/// couldn't complete (hung Arc killed at the budget, or spawn failure) and the +/// last good value must be KEPT — publishing nil there would transiently drop a +/// valid excluded-site URL and let a denylisted page through (W-557). enum BrowserURLResolution: Sendable { case resolved(URL?) case unavailable @@ -345,11 +360,22 @@ final class BrowserURLCache { } } - // osascript subprocess so it can be killed; temp file instead of Pipe to avoid the 64 KB buffer deadlock. + /// `URL of active tab of front window` via an `osascript` subprocess. Shelling + /// out (rather than in-process `NSAppleScript`) lets this run off the main + /// thread and, crucially, be *killed* if Arc is unresponsive — `NSAppleScript` + /// offers no such escape hatch. TCC attributes the Apple Event to Mojito (the + /// responsible process), so the one-time Automation grant is the same as + /// before. Output goes to a temp file, not a `Pipe`: a `Pipe`'s 64 KB buffer + /// can fill and block `osascript`'s write so it never exits, manufacturing a + /// timeout on an otherwise-responsive resolve. Returns `.resolved` on a clean + /// exit (URL or nil), `.unavailable` on spawn failure or the hung-Arc kill so + /// the caller keeps the last good value. Runs on `resolveQueue`, off main. private nonisolated static func osascriptURL(bundleID: String) -> BrowserURLResolution { let outURL = URL(fileURLWithPath: NSTemporaryDirectory()) .appendingPathComponent("mojito-arcurl-\(UUID().uuidString)") FileManager.default.createFile(atPath: outURL.path, contents: nil) + // Install the cleanup BEFORE the FileHandle guard so a failed open can't + // leak the temp file. defer { try? FileManager.default.removeItem(at: outURL) } guard let outHandle = try? FileHandle(forWritingTo: outURL) else { return .unavailable } @@ -372,7 +398,13 @@ final class BrowserURLCache { return .unavailable // keep the last good value } try? outHandle.close() - // Clean exit: read the file. A non-URL / empty output is a real "no URL". + // A nonzero exit is an AppleScript/TCC/window error (e.g. "no active tab" + // when the front window is a transient Little-Arc preview), NOT a + // confirmed "no URL". Treat it as unavailable so a valid cached + // excluded-site URL survives a transient error rather than being cleared + // (which would let a denylisted page through). Only a clean exit with + // empty output is a real `resolved(nil)`. + guard process.terminationStatus == 0 else { return .unavailable } let raw = ((try? String(contentsOf: outURL, encoding: .utf8)) ?? "") .trimmingCharacters(in: .whitespacesAndNewlines) return .resolved(raw.isEmpty ? nil : BrowserURL.normalizedURL(from: raw)) diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index db6874f..fb3cbe8 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -24,10 +24,28 @@ final class FocusedElementCache { /// switch, seed install, within-app focus move) invalidates the cached /// field info below — it described the *previous* focus. private(set) var element: AXUIElement? { - didSet { haveFieldInfo = false } + didSet { + haveFieldInfo = false + _ = fieldGeneration.withLock { $0 += 1 } // cancel in-flight reclassifies + } } - // Cached per focus to skip repeated AX IPC on the tap thread; reset on any element change (see element.didSet). + /// Bumped on every `element` change so a queued off-thread reclassify can bail + /// before its (up to `seedTimeout`) AX round-trips instead of piling up a + /// serial backlog under a focus storm. Lock-protected so `reclassify` can read + /// it from the seed queue. + private let fieldGeneration = OSAllocatedUnfairLock(initialState: 0) + + /// Cached "is this field secure / editable" answers for `element`, so the + /// tap-path context builder (`AppContextDetector.current`) does **zero** AX + /// attribute IPC — deriving them is several synchronous cross-process + /// round-trips, and doing that on the tap thread against a hung app trips the + /// event-tap timeout and drops keystrokes (W-557). They're computed **off the + /// main thread** alongside the element (seed + focus-change reclassify), so + /// `current()` only reads them. `haveFieldInfo` is false until the classify + /// lands; `current()` treats that as "secure, not editable" (fail closed), so + /// an unclassified field never begins capture. Reset to unknown whenever + /// `element` changes (see `element.didSet`). private(set) var focusedIsSecure = false private(set) var focusedIsEditable = false private(set) var haveFieldInfo = false @@ -44,8 +62,21 @@ final class FocusedElementCache { /// Classifies `element` on the seed queue (off the tap thread) and publishes /// the result iff focus hasn't moved on. Used for within-app focus moves; the /// initial app-switch classify rides the seed round-trip in `seed`. + /// + /// Note the inherent limit of non-blocking focus tracking: between a focus + /// move and this landing, `current()` fails closed (haveFieldInfo is cleared + /// by `element.didSet`), so it can't leak. But if the app's AX observer never + /// registered (`seed` best-effort), within-app focus moves aren't seen at all + /// and a stale-but-authoritative classification can persist until the next app + /// activation re-seeds. Fully closing that needs synchronous re-verification — + /// exactly the tap-blocking IPC this whole change removes — so for AX-opaque + /// apps the app/URL exclusion list remains the backstop for password contexts + /// (same as the secure-field detection has always relied on). private nonisolated func reclassify(_ element: AXUIElement) { + let generation = fieldGeneration.withLock { $0 } Self.seedQueue.async { + // Focus already moved on again → skip the round-trips entirely. + guard self.fieldGeneration.withLock({ $0 }) == generation else { return } AXUIElementSetMessagingTimeout(element, Self.seedTimeout) let info = AppContextDetector.classify(element) DispatchQueue.main.async { diff --git a/Tests/MojitoTests/BrowserURLCacheTests.swift b/Tests/MojitoTests/BrowserURLCacheTests.swift index eb1a987..bcf34d9 100644 --- a/Tests/MojitoTests/BrowserURLCacheTests.swift +++ b/Tests/MojitoTests/BrowserURLCacheTests.swift @@ -176,18 +176,27 @@ struct BrowserURLCacheTests { @Test func unavailableResolveKeepsLastGoodValue() async throws { // A resolve that times out against a hung Arc returns `.unavailable`; it // must NOT erase a previously-cached URL (that would transiently drop a - // denylisted-site URL and let an excluded page through). + // denylisted-site URL and let an excluded page through). Clock-driven + // throttling keeps the reads deterministic — no spurious reschedules. let spy = ResolverSpy(); spy.setStub(URL(string: "https://kept.example")) - let cache = makeCache(spy: spy, clock: Clock(Date()), minRefreshInterval: 0) + let clock = Clock(Date(timeIntervalSince1970: 5_000)) + let cache = makeCache(spy: spy, clock: clock, minRefreshInterval: 1.0) + // Warm: the first read resolves and caches "kept". Reads within the 1s + // window are throttled, so `spy.calls` stays 1. _ = cache.url(forBundleID: Self.arc, pid: 42) + #expect(try await settle { spy.calls == 1 }) #expect(try await settle { cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://kept.example") }) - // Next refresh comes back unavailable (hung); the cached value survives. + // Past the window, the next resolve comes back unavailable (hung/killed). + clock.now += 1.1 spy.setOutcome(.unavailable) - _ = cache.url(forBundleID: Self.arc, pid: 42) // schedules a refresh + _ = cache.url(forBundleID: Self.arc, pid: 42) // schedules refresh #2 (.unavailable) #expect(try await settle { spy.calls == 2 }) - try await Task.sleep(nanoseconds: 20_000_000) // let the publish land + try await Task.sleep(nanoseconds: 30_000_000) // let the publish land + + // The cached value survived. This read is within the new throttle window + // (lastRefreshAt advanced), so it doesn't reschedule and can't flake. #expect(cache.url(forBundleID: Self.arc, pid: 42) == URL(string: "https://kept.example")) } diff --git a/scripts/e2e/hung-app-typing-regression.sh b/scripts/e2e/hung-app-typing-regression.sh index fd76d74..863cce4 100755 --- a/scripts/e2e/hung-app-typing-regression.sh +++ b/scripts/e2e/hung-app-typing-regression.sh @@ -176,7 +176,17 @@ run_target() { fi drive hotkey command l # focus a text field (URL bar / doc) sleep 0.2 + local tpid; tpid="$(pgrep -x "$name" | head -1)" + [ -n "$tpid" ] || { fail "$name is not running — can't freeze it; inconclusive."; exit 2; } for p in $(pgrep -x "$name"); do kill -STOP "$p"; done # freeze: no IPC answers + # PROVE it actually froze — a failed STOP would leave the target responsive + # and a clean log would be a false pass. `T` = stopped. + local st; st="$(ps -o state= -p "$tpid" 2>/dev/null | tr -d ' ')" + case "$st" in + T*) ;; + *) for p in $(pgrep -x "$name"); do kill -CONT "$p" 2>/dev/null; done + fail "$name did not freeze (state '${st:-gone}') — inconclusive."; exit 2 ;; + esac drive type "$PHRASE " # each word+space fires detect() sleep "$FREEZE_SECS" # hold: a blocking tap path times out here for p in $(pgrep -x "$name"); do kill -CONT "$p"; done # thaw From 57ccdbf223c647cbbb1a78f3d194dfa9372dead7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:25:31 +0000 Subject: [PATCH 8/8] trim remaining multi-line comment blocks to one-liners (CLAUDE.md style) Refs: W-557 Co-Authored-By: Claude Sonnet 4.6 --- Sources/Mojito/Context/BrowserURL.swift | 26 +++---------------- .../Mojito/Context/FocusedElementCache.swift | 25 ++---------------- 2 files changed, 5 insertions(+), 46 deletions(-) diff --git a/Sources/Mojito/Context/BrowserURL.swift b/Sources/Mojito/Context/BrowserURL.swift index c681949..60ffb3f 100644 --- a/Sources/Mojito/Context/BrowserURL.swift +++ b/Sources/Mojito/Context/BrowserURL.swift @@ -30,14 +30,7 @@ enum BrowserURL { } let app = AXUIElementCreateApplication(pid) - // This DFS walk runs on the tap thread (main); each node is a synchronous - // AX round-trip into the browser. Two bounds keep a hung/slow browser from - // stalling the tap past its ~1s limit and dropping the keystroke (W-557): - // every element queried is pinned to `walkAXTimeout` (a per-object setting - // — a timeout on `app` does NOT propagate to its descendants), and the - // whole walk is capped by `walkDeadline` so a tree of individually-fast- - // but-collectively-slow nodes still can't run long. A frozen app fails the - // first query and aborts immediately. + // Two bounds: per-object timeout (does not propagate to descendants) + total deadline (caps a deep tree of fast-but-numerous nodes). let deadline = Date().addingTimeInterval(Self.walkBudget) // Most browsers expose the page URL as an `AXURL` attribute somewhere @@ -201,11 +194,7 @@ enum BrowserURL { } } -/// Outcome of one URL resolve. `resolved(nil)` is a real "no URL" answer (no -/// window / non-URL value) and is cached as such; `unavailable` means the resolve -/// couldn't complete (hung Arc killed at the budget, or spawn failure) and the -/// last good value must be KEPT — publishing nil there would transiently drop a -/// valid excluded-site URL and let a denylisted page through (W-557). +/// `.unavailable` means the resolve timed out — callers must keep the last good value, not overwrite it with nil. enum BrowserURLResolution: Sendable { case resolved(URL?) case unavailable @@ -360,16 +349,7 @@ final class BrowserURLCache { } } - /// `URL of active tab of front window` via an `osascript` subprocess. Shelling - /// out (rather than in-process `NSAppleScript`) lets this run off the main - /// thread and, crucially, be *killed* if Arc is unresponsive — `NSAppleScript` - /// offers no such escape hatch. TCC attributes the Apple Event to Mojito (the - /// responsible process), so the one-time Automation grant is the same as - /// before. Output goes to a temp file, not a `Pipe`: a `Pipe`'s 64 KB buffer - /// can fill and block `osascript`'s write so it never exits, manufacturing a - /// timeout on an otherwise-responsive resolve. Returns `.resolved` on a clean - /// exit (URL or nil), `.unavailable` on spawn failure or the hung-Arc kill so - /// the caller keeps the last good value. Runs on `resolveQueue`, off main. + // osascript subprocess: runs off main, killable if Arc hangs. Temp file instead of Pipe avoids the 64 KB buffer deadlock. private nonisolated static func osascriptURL(bundleID: String) -> BrowserURLResolution { let outURL = URL(fileURLWithPath: NSTemporaryDirectory()) .appendingPathComponent("mojito-arcurl-\(UUID().uuidString)") diff --git a/Sources/Mojito/Context/FocusedElementCache.swift b/Sources/Mojito/Context/FocusedElementCache.swift index fb3cbe8..a2905e0 100644 --- a/Sources/Mojito/Context/FocusedElementCache.swift +++ b/Sources/Mojito/Context/FocusedElementCache.swift @@ -36,16 +36,7 @@ final class FocusedElementCache { /// it from the seed queue. private let fieldGeneration = OSAllocatedUnfairLock(initialState: 0) - /// Cached "is this field secure / editable" answers for `element`, so the - /// tap-path context builder (`AppContextDetector.current`) does **zero** AX - /// attribute IPC — deriving them is several synchronous cross-process - /// round-trips, and doing that on the tap thread against a hung app trips the - /// event-tap timeout and drops keystrokes (W-557). They're computed **off the - /// main thread** alongside the element (seed + focus-change reclassify), so - /// `current()` only reads them. `haveFieldInfo` is false until the classify - /// lands; `current()` treats that as "secure, not editable" (fail closed), so - /// an unclassified field never begins capture. Reset to unknown whenever - /// `element` changes (see `element.didSet`). + // Cached per focus to skip AX IPC on the tap thread; reset on element change (see element.didSet), fail-closed until set. private(set) var focusedIsSecure = false private(set) var focusedIsEditable = false private(set) var haveFieldInfo = false @@ -59,19 +50,7 @@ final class FocusedElementCache { haveFieldInfo = true } - /// Classifies `element` on the seed queue (off the tap thread) and publishes - /// the result iff focus hasn't moved on. Used for within-app focus moves; the - /// initial app-switch classify rides the seed round-trip in `seed`. - /// - /// Note the inherent limit of non-blocking focus tracking: between a focus - /// move and this landing, `current()` fails closed (haveFieldInfo is cleared - /// by `element.didSet`), so it can't leak. But if the app's AX observer never - /// registered (`seed` best-effort), within-app focus moves aren't seen at all - /// and a stale-but-authoritative classification can persist until the next app - /// activation re-seeds. Fully closing that needs synchronous re-verification — - /// exactly the tap-blocking IPC this whole change removes — so for AX-opaque - /// apps the app/URL exclusion list remains the backstop for password contexts - /// (same as the secure-field detection has always relied on). + // Classifies off-thread; publishes iff focus hasn't moved. AX-opaque apps that miss within-app moves rely on the exclusion list as backstop. private nonisolated func reclassify(_ element: AXUIElement) { let generation = fieldGeneration.withLock { $0 } Self.seedQueue.async {