W-556: Arc typing E2E regression harness + os_log E2E hook#176
Conversation
Typing a word then a terminator (space/./,/!/?) in Arc's command bar stalled: the first press after the word did nothing, a second was needed. Cause: an ambient-emoticon terminator (`<3`→❤️ check) makes Engine.process call AppContextDetector.current() for a fresh per-site exclusion check, which calls BrowserURL.detect. Arc hides its web a11y tree, so every AX path returns nil and detect fell through to a synchronous NSAppleScript round-trip to Arc. That whole chain runs *inside the CGEventTap callback* (synchronous on the main run loop). A busy Arc replies slowly, the callback blows past the tap timeout, macOS fires tapDisabledByTimeout and drops the keystroke. Only Arc hit this — it's the sole browser on the AppleScript path. Same failure class as the W-547 typing stall. Fix: never run the Apple Event on the main/tap thread. New BrowserURLCache resolves Arc's URL on a background serial queue and serves the last value synchronously; detect() returns the cached URL and schedules a refresh for next time. Prefetches on app activation, and only serves a URL for the pid it was resolved from so a stale value can't cross an app switch. Mirrors FocusedElementCache's off-main-seed pattern. Staleness is bounded to about one focus-change/keystroke, fine for per-site exclusion matching, and it fixes all six BrowserURL.detect call sites, not just the emoticon one. Verified: Debug build succeeds; full 266-test suite green. Refs: W-555
Cross-provider (Codex) review of the first cut found the async cache moved NSAppleScript to a background GCD queue, but NSAppleScript is documented main-thread-only — off-main it can hang or misbehave, which would also wedge the single-flight `refreshing` flag forever. It also left the synchronous AX tree walk on the tap thread for Arc even though that walk always returns nil there. Rework: - detect(): for AppleScript-only browsers (Arc) branch straight to the cache before the AX walk, so no cross-process AX IPC runs on the tap thread for Arc at all. - BrowserURLCache: run NSAppleScript on the MAIN thread (where it's supported) but via DispatchQueue.main.async so it lands on its own run-loop turn, never inside the CGEventTap callback. A 1s throttle keeps refreshes from recurring per keystroke; app-activation refreshes bypass it. The scheduled block always clears `refreshing`, so a failed/timed-out script can't wedge the cache (fixes the stuck-single-flight finding). Hot-path read still does zero IPC — returns cached, schedules a refresh. pid-guarded so a stale URL can't cross an app switch. Verified: Debug build succeeds; full suite green (266 tests, 20 suites). Refs: W-555
Add a test seam to BrowserURLCache (inject resolver + clock, skip the workspace observer) and a BrowserURLCacheTests suite that pins the property whose violation caused the bug: the hot-path read does no blocking IPC and defers the AppleScript to a later main-thread turn. Covered deterministically (no live Arc / AppleScript / app switch): - cold read returns nil and does NOT resolve synchronously — the resolver runs once, on a later turn, on the main thread (regression guard for AppleScript creeping back onto the tap thread) - cached value served for the same pid; withheld across a pid change - a burst of reads collapses to one resolve (single-flight) - throttle skips a refresh within the interval, allows it after (injected clock) - hot-path read returns in <50ms even when the resolver blocks 200ms The true end-to-end "keys don't drop in Arc" is timing/Arc-dependent and can't be asserted deterministically; these nail the cause instead. Suite is green: 273 tests, 21 suites. Refs: W-555
Arc keeps generating bug reports (W-555 latest) and its event-tap / AX
behavior can't be exercised by unit tests. Add a live end-to-end harness
that drives real Arc + a real Mojito build and asserts the tap stays
healthy.
- DebugRecorder: when launched with MOJITO_E2E_LOG=1, mirror every recorded
activity event into the unified log (subsystem ee.wells.Mojito, category
e2e). Off by default — the logger isn't even constructed without the env
var — so production is untouched. Values are already ASCII-clamped, safe
to log %{public} so `log show` can read them back. Makes the tapLost /
focus / picker stream queryable headlessly.
- scripts/e2e/arc-typing-regression.sh: relaunch Mojito Dev with the flag,
type terminator-heavy phrases into fresh Arc tabs, assert zero
`keyMonitor tapLost reason=timeout`. Exit 0 healthy / 1 bug / 2
inconclusive (no e2e activity is reported, never a false pass).
- scripts/e2e/cgtype.swift: HID-level CGEvent keystroke helper. Critical:
AppleScript `keystroke` does NOT traverse Mojito's session tap, so it
can't drive the code under test; HID CGEvents do (verified — HID `:tada`
fires engine colon / picker open, System Events fires nothing).
- Docs: scripts/e2e/README.md + a CLAUDE.md Testing subsection, incl. the
gotchas (HID events, /usr/bin/log vs the zsh builtin, launchctl+open vs
direct-exec).
Validated on this machine as far as possible without Arc installed: the
os_log hook emits real events, /usr/bin/log capture works, the dev build's
TCC grants are live, and cgtype-driven keystrokes reach Mojito's engine
(engine colon / picker open) with zero tap timeouts on a healthy path. The
Arc-specific new-tab run needs a Mac with Arc. Unit suite green (273/21).
Refs: W-556
![wr-claude-reviewer[bot]](https://avatars.githubusercontent.com/in/3849589?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean addition — the DebugRecorder os_log hook is well-scoped (env-gated, no production cost, Logger? stays nil unless MOJITO_E2E_LOG=1), the "no false pass" inconclusive path is a good safety property, and the HID-vs-AppleScript distinction is well documented. No blocking findings.
Nits (all non-blocking):
scripts/e2e/arc-typing-regression.sh — Arc preflight mdfind fallback is a dead branch.
Line 67:
if ! open -Ra "Arc" 2>/dev/null && ! mdfind "kMDItemCFBundleIdentifier == '$ARC_BUNDLE'" | grep -q .; thenWhen Arc is absent, mdfind produces no output → grep -q . exits 1 → with the leading ! that evaluates to FALSE, so the && never enters the fail branch via the mdfind path. (The condition as written is true only when open fails and mdfind succeeds — a contradiction.) Practical impact is limited because ARC_SEEN -eq 0 in the analysis phase still exits 2, just with the less-informative "inconclusive" message instead of "Arc not installed". A cleaner alternative:
if ! (open -Ra "Arc" 2>/dev/null || mdfind "kMDItemCFBundleIdentifier == '$ARC_BUNDLE'" | grep -q .); thenscripts/e2e/cgtype.swift — modifier name lookup is case-sensitive with a silent fallback.
Line 65: the dictionary key lookup uses args[2] as-is, so Command vs command silently falls through to ?? [] (no modifier) instead of erroring. args[3] already calls .lowercased() for the key — applying the same to args[2] would make it consistent and prevent a silent misbehave:
["command": .maskCommand, "shift": .maskShift, "option": .maskAlternate, "control": .maskControl][args[2].lowercased()] ?? []Cross-provider (Codex) review flagged that the harness could report a green
run while testing nothing. Fixes:
- Positive keystroke proof (was: "saw an Arc log line"). Activating Arc alone
emits a focus event, so that was no proof anything typed. Now the harness
probes with a `:` trigger and REQUIRES an `engine colon` event before
trusting the run; no probe hit → inconclusive (exit 2), never a pass.
- Build from this worktree by default (--no-build/--app to override) so a
regressed-but-unrebuilt binary can't sneak a stale green through.
- Fail hard on driver errors: validate --cycles is a positive int; cgtype
now exits non-zero if it can't post an event, and every drive step is
checked (a silent no-op was the other route to a false pass).
- Read the log via `log show` over a recorded time window instead of a
streamed pipe — no attach race, no truncated tail.
- Preserve/restore the prior MOJITO_E2E_LOG session value in cleanup.
- DebugRecorder: correct the comment — the ASCII clamp bounds spillage but
is NOT anonymization; %{public} is safe only because call sites pass
non-sensitive metadata.
Validated: `log show` capture + the probe detect `engine colon`; cgtype
compiles and reports failures; bad --cycles and missing Arc exit 2. Unit
suite green (273/21). The Arc-specific run still needs a Mac with Arc.
Refs: W-556
There was a problem hiding this comment.
Round 2 follow-up — previous findings addressed / no new blocking issues.
The prior round APPROVED with two nits. The new commit (5b39ae1) adds the probe guardrail; no prior blocking findings to carry over.
On prior nit 1 — Arc preflight mdfind logic: re-examining this, the bash grammar makes ! cmd1 | cmd2 negate the exit status of the entire pipeline cmd1 | cmd2, not just cmd1. When Arc is absent, mdfind | grep -q . exits 1 (grep gets no input), ! flips it to 0 (true), so ! open -Ra "Arc" && ! mdfind ... | grep -q . correctly enters the fail branch. The prior nit was based on a wrong parse; the code is correct as written.
On prior nit 2 — cgtype.swift modifier case-sensitivity: still not addressed (no .lowercased() on args[2]), but inconsequential in practice — every drive hotkey call in the harness passes a literal lowercase string.
New in this commit — the probe step: solid addition. Requiring engine colon before proceeding is exactly the right guard — it distinguishes "Mojito saw keystrokes" from "Mojito is merely running." The exit 2 inconclusive path on probe failure correctly prevents a false green. No issues with the implementation.
1 participant
Stacked on #175 (base is the W-555 branch, so this PR's diff is just the harness). Retarget to
mainonce #175 merges.Arc keeps generating bug reports and its event-tap/AX behavior can't be exercised by unit tests. This adds a live E2E harness that drives real Arc + a real Mojito build and asserts the tap stays healthy.
Changes
DebugRecorder): withMOJITO_E2E_LOG=1, mirror every recorded activity event into the unified log (ee.wells.Mojito/e2e). Off by default (logger not even constructed) → production untouched. Values are ASCII-clamped, safe to log%{public}. Makes the tapLost/focus/picker stream queryable headlessly.scripts/e2e/arc-typing-regression.sh: relaunch Mojito Dev with the flag, type terminator-heavy phrases into fresh Arc tabs, assert zerokeyMonitor tapLost reason=timeout. Exit 0 healthy / 1 bug / 2 inconclusive (no-activity is reported, never a false pass).scripts/e2e/cgtype.swift: HID-level CGEvent keystroke helper. AppleScriptkeystrokedoes NOT traverse Mojito's session tap (verified — it drives nothing); HID CGEvents do (:tada→engine colon/picker open).scripts/e2e/README.md+ CLAUDE.md Testing subsection, with the gotchas (HID events,/usr/bin/logvs the zsh builtin,launchctl setenv+openvs direct-exec).Validation
Confirmed on this machine as far as possible without Arc installed: the os_log hook emits real events,
/usr/bin/logcapture works, the dev build's TCC grants are live, andcgtype-driven keystrokes reach Mojito's engine (engine colon/picker open) with zero tap timeouts on a healthy path. The Arc-specific new-tab run needs a Mac with Arc. Unit suite green (273/21).Refs: W-556