[TimeToBuildBob]

Summary

Rebase of #141 (hawai-i) onto current master after #145. Original work is preserved and attributed (Co-Authored-By: hawai-i).

Browser URL/title enrichment is optional, but the macOS Swift watcher used to return without a heartbeat when Chrome/Safari ScriptingBridge failed. Heartbeat merging then extended the previous application across the real browser interval.

This keeps #141's PID-based foreground reconciliation:

• emit a foreground heartbeat even when browser context acquisition fails
• bind tracked app, AX observer, focused window, and emitted app identity to one PID
• repair a missing/stale observer and reject stale AX callbacks
• consume the activated NSRunningApplication from the workspace notification

Conflict with #145

#145 landed on master after #141 opened. The rebase keeps that work:

• axString / axElement CF type checks (the macOS: native helper crashes when NSHTTPURLResponse is bridged as a window-title string #144 NSHTTPURLResponse crash)
• observer teardown that unregisters both title and focused-window notifications and tracks observedApp (the [macOS] aw-watcher-window-macos leaks AXObserver/Mach ports, causing high wired memory #139 Mach-port leak)

Unsafe as? String / as! AXUIElement casts from #141 are not carried forward.

Greptile on #141

P1 (incognito): ScriptingBridge is the only Chrome incognito detector. The original fallback sent AX-derived app + title when that lookup failed. The rebase still emits the Chrome app identity (so tracking stays coherent) but drops title and URL. Successful incognito detection still blanks both app and title.

P2 (tests): make test-swift now compiles with -target …-macosx$(MACOSX_DEPLOYMENT_TARGET) (12.0), matching the production helper.

Validation

Python tests: 77 passed. mypy clean. Swift helper/tests need Darwin CI (macos-latest).

Related

• macOS: keep foreground tracking coherent when browser context fails #141 — original PR (leave open until this lands)
• fix(macos): survive non-string AX titles and stop leaking AXObserver #145 / macOS: native helper crashes when NSHTTPURLResponse is bridged as a window-title string #144 / [macOS] aw-watcher-window-macos leaks AXObserver/Mach ports, causing high wired memory #139 — AX title crash and observer leak, now on master

[TimeToBuildBob]

macos-latest failed at make build-swift: compiling macos.swift together with macos_state.swift rejects top-level expressions (formatter.formatOptions = …, start(), RunLoop.main.run()).

The rebase onto master after #145 dropped the #141 @main wrapper and closure inits. Restored in 8ac81d3. CI is re-running.

[TimeToBuildBob]

CI-green and mergeable — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

[TimeToBuildBob]

🤖 AI code review

This PR refactors the macOS Swift watcher to keep foreground tracking when browser context (ScriptingBridge) fails, binding tracked app, AX observer, focused window, and emitted app identity to one PID. It adds a new macos_state.swift file with pure state-transition helpers and a Swift test suite, plus Python source-level wiring tests. The Makefile now compiles macos_state.swift into the production helper and adds a test-swift target.

Needs a look — P2 only

Confidence 4/5

_{ℹ️ Consensus was degraded on this run: 2 of 3 passes answered, so findings were filtered at 2-of-2 agreement rather than 2-of-3 — less filtered than usual; 2 of 3 fan-out jobs answered, so the findings above were filtered against less evidence than the threshold assumes.}

1 finding · ⚠️ 1 P2 — non-blocking (round cap reached)

⚠️ P2 medium · ℹ️ non-blocking — aw_watcher_window/macos.swift:989

In emitHeartbeat, the Chrome/Safari ScriptingBridge failure fallback drops the AX title and URL, but the success path for Chrome incognito sets data = NetworkMessage(app: "", title: "") — which drops the app identity entirely. The PR description says 'Successful incognito detection still blanks both app and title', but the code also blanks the app, so the heartbeat for an incognito Chrome window has app="" and title="". This is intentional per the comment, but it means the foreground tracking is not coherent for incognito: the heartbeat has no app identity, and the server-side merge may treat it as a gap or as an unknown app. The PR's stated goal of keeping foreground tracking coherent when browser context fails is not met for the incognito case, because the app identity is dropped. This is a behavior change from the previous code which also blanked app and title for incognito, so it is not a regression, but it is a contradiction with the PR's stated design. The finding is that the fallback path keeps app identity, but the incognito path does not, so the two paths are inconsistent: a user in incognito Chrome gets a heartbeat with app="" which may be filtered or displayed as an empty app, while a user in a normal Chrome window with SB failure gets app="Google Chrome". The observable consequence is that incognito browsing is not tracked as a gap but as an empty-app event, which may be merged or displayed incorrectly. This is a real defect in the new logic because the PR claims to keep foreground tracking coherent, but the incognito branch still drops the app identity.

Consider keeping the app identity in incognito mode as well, e.g., data = NetworkMessage(app: applicationName, title: "") — but note that the original behavior blanked app to hide incognito activity. If the goal is to keep tracking coherent, the app identity should be kept; if the goal is to hide incognito, then the fallback should also blank the app. The inconsistency is the issue.

_{How this was verified: Checked the emitHeartbeat function: the incognito branch at line 989 sets app to empty string, while the fallback at line 1001 keeps app identity. The PR description says 'Successful incognito detection still blanks both app and title', confirming this is intentional, but it contradicts the PR's goal of coherent foreground tracking.}

_{Consensus: 2/2 passes agreed}
Distinct keys: 1 (general)

1 advisory finding (summary-only, not scored)

These P2 guard, heuristic, trade-off, or documentation claims are retained for judgment without opening review threads.

⚠️ P2 medium · ℹ️ non-blocking — aw_watcher_window/macos.swift:842

In rebuildObserver, when AXObserverAddNotification fails, the code sets observer = newObserver and then observer = nil to release the observer. However, the comment says 'AXObserver is Swift-ARC-managed (see the Create-overwrite warning on tearDownObserver). Parking it on observer and nilling is the release; CFRelease would double-free.' This is questionable: AXObserver is a CFType, and Swift does not ARC-manage CF types unless they are wrapped in a Swift type. The variable newObserver is a Swift optional of type AXObserver?, which is a CF type. Assigning it to observer and then nil'ing it may not release the CF object; it may leak. The previous code in tearDownObserver relied on niling the observer to release it, but that is also questionable. The real issue is that when the add fails, the observer is never added to the run loop, so the run loop source is not retained, but the observer object itself may leak because it is not CFReleased. The comment claims ARC manages it, but AXObserver is a CFType and Swift does not automatically manage CFType memory unless it is bridged to a Swift class. This could be a memory leak, but it is a pre-existing pattern. The new code adds this pattern in the failure path, which is new. The consequence is a potential leak of the AXObserver object on every failed observer creation, which could happen repeatedly if the app is not accessible. This is a real issue but low severity.

If the observer is not installed, consider calling CFRelease(newObserver) after niling, but verify the memory management semantics. Alternatively, keep the observer in a local variable and let it go out of scope without assigning to the property.

_{How this was verified: Checked the code around lines 836-844. The comment says 'AXObserver is Swift-ARC-managed' but AXObserver is a CFType; Swift does not ARC-manage CF types. The local newObserver is a strong reference, and niling the property does not release the CF object. The original tearDownObserver also niled without CFRelease, but that was for an observer that was added to the run loop; here it was never added, so the only reference is the local. This is a leak on every failed add.}

_{Consensus: 2/2 passes agreed}
Distinct keys: 1 (general)

Files changed (5) — the diff as I read it

• Makefile — Adds macos_state.swift to the swiftc build, adds test-swift target, and runs it on Darwin in test.
• aw_watcher_window/macos.swift — Refactors MainThing to track foregroundApplication, reconcile foreground app on poll/activation, rebuild AX observer, and emit heartbeats even when browser context fails.
• aw_watcher_window/macos_state.swift — New file with pure functions for foreground reconciliation, AX callback filtering, title registration retry, and browser fallback heartbeat.
• tests/macos_state_tests.swift — New Swift test executable covering the pure state functions in macos_state.swift.
• tests/test_macos_heartbeat_wiring.py — New Python source-level tests asserting exclusion ordering and heartbeat delivery wiring in macos.swift.

Previous review passes

commit	score	findings	engine	when
8ac81d39c2f8	4/5	2	llm	2026-09-16 08:53 UTC
9424b4f6a054	1/5	3	llm	2026-09-16 12:44 UTC
90aad7fbb413	4/5	1	llm	2026-09-16 13:25 UTC
fe2f5c1c7651	3/5	2	llm	2026-09-25 11:24 UTC
194d002c6890	5/5	0	llm	2026-10-07 14:45 UTC
194d002c6890	4/5	1	llm	2026-10-09 04:18 UTC
2fb21ed97546	5/5	0	llm	2026-10-14 18:02 UTC

_{Reviewed 2fb21ed97546 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 1204s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}

[TimeToBuildBob]

@greptileai review

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; no outstanding correctness, security, or repository-rule issue was identified.

Findings

1.  Title registration is not retried ▶

Summary

This PR makes the macOS native watcher keep foreground application tracking coherent when optional browser context acquisition fails.

• Reconciles the foreground application, accessibility observer, focused window, and emitted identity by PID.
• Repairs missing observers and rejects stale accessibility callbacks.
• Emits privacy-safe browser fallback heartbeats without a title or URL when browser scripting fails.
• Adds focused state tests and runs them on Darwin using the supported macOS 12 deployment target.
• The previous title-notification retry finding is fully fixed: failed registration leaves the state retryable on the next poll.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Startup, activation, or poll] --> B[Select live foreground application]
    B --> C{PID changed or observer missing?}
    C -->|Yes| D[Rebuild AX observer]
    C -->|No| E[Reuse observer]
    D --> F[Read focused window]
    E --> F
    F --> G[Register or retry title notification]
    G --> H{Supported browser?}
    H -->|Chrome or Safari| I[Attempt browser context lookup]
    H -->|Firefox| J[Read URL from AX tree]
    H -->|Other app| K[Keep app and AX title]
    I -->|Success| L[Use browser title and URL]
    I -->|Failure| M[Keep app; blank title and URL]
    J --> N[Apply privacy filters]
    K --> N
    L --> N
    M --> N
    N --> O[Emit foreground heartbeat]

Loading

_{Reviews (2) · Last reviewed commit: "fix(macos): retry title-change observer ..."}

[TimeToBuildBob]

@greptileai review

[TimeToBuildBob]

Rebased onto master (194d002) — conflict was with #152 in macos.swift.

This PR moves browser scripting out of the old inline block into emitHeartbeat(), so I took the PR's structure and ported #152's behavior there: Chrome/Safari now address ScriptingBridge by processIdentifier (no bundle-ID relaunch) and return early if the app isTerminated, before the context-failure fallback can emit a heartbeat for a browser that already quit. #153's Gecko browser list came through without conflicts.

Python tests pass locally (104); the Swift build/state tests run on macOS CI.

[TimeToBuildBob]

Conflict rebase blocked — architectural incompatibility with recent master changes.

What landed on master after the last rebase (194d002, Oct 6):

• 6d58cef (fix(macos): serialize heartbeats through a retrying queue, create the bucket lazily #162, Oct 10): replaced var oldHeartbeat / func sendHeartbeat / func sendHeartbeatSingle with HeartbeatSender — a queued, retrying async class with dropForExclusion()
• 1d07782 (feat(config): add exclude_apps option to suppress logging for matched apps #156, Oct 14): added appShouldBeExcluded() and wired it into reconcileForegroundApp via heartbeatSender.dropForExclusion()
• e275219 (fix(main): don't exit when aw-server starts slowly at boot #164, Oct 14): startup fix

Why it's not a simple rebase:

This PR's restructuring of reconcileForegroundApp (observer-only, heartbeat removed) and HeartbeatSender (#162) both rewrote the heartbeat delivery path, but in orthogonal directions. The conflict isn't syntactic — it's two independent refactors of the same mechanism.

What the clean merge needs:

1. Keep the PR's observer management: foregroundReconciliationAction, rebuildObserver, refreshFocusedWindow, the trimmed reconcileForegroundApp
2. Keep master's HeartbeatSender (queue, retry, dropForExclusion) — don't regress fix(macos): serialize heartbeats through a retrying queue, create the bucket lazily #162
3. Wire appShouldBeExcluded (from feat(config): add exclude_apps option to suppress logging for matched apps #156) into the heartbeat path that uses HeartbeatSender
4. Remove the old var oldHeartbeat / func sendHeartbeat / func sendHeartbeatSingle globals (already gone in master, but this PR still has them at 194d002)

This needs a dedicated Swift session to produce the unified version. Filing a task.

[TimeToBuildBob]

Resolved the conflict and force-pushed the rebase onto current master (aebd113), new head 2fb21ed.

• Kept this PR's PID-based foreground/observer reconciliation and privacy-safe Chrome/Safari context-failure fallback.
• Kept master's HeartbeatSender and send helpers byte-for-byte: queued delivery, bucket creation, retry/backoff and exclusion merge-break behavior are unchanged (fix(macos): serialize heartbeats through a retrying queue, create the bucket lazily #162).
• Moved the app-exclusion check into emitHeartbeat(), before title acquisition or browser scripting, so feat(config): add exclude_apps option to suppress logging for matched apps #156 remains enforced on polling, activation and AX callbacks.
• Master's startup and Gecko changes are retained.

Added Linux-runnable source-wiring regression checks; the exclusion check failed before the fix and passes afterward. In an isolated worktree environment: 160 Python tests + 11 subtests passed, mypy clean, diff whitespace check clean. Native Swift compilation/state tests still require macOS CI; no local native-build claim. GitHub now reports the PR mergeable, with fresh CI running.

Also resolved the remaining repeated nil-window observer finding: removal is outside the optional new-window registration, and the existing Swift state test explicitly covers that path. No observer behavior change was needed.

[TimeToBuildBob]

⚠️ P2 — In emitHeartbeat, the Chrome/Safari ScriptingBridge failure fallback drops the AX title and URL, but the success path for Chrome incognito sets data = NetworkMessage(app: "", title: "") — which drops the app identity entirely. The PR description says 'Successful incognito detection still blanks both app and title', but the code also blanks the app, so the heartbeat for an incognito Chrome window has app="" and title="". This is intentional per the comment, but it means the foreground tracking is not coherent for incognito: the heartbeat has no app identity, and the server-side merge may treat it as a gap or as an unknown app. The PR's stated goal of keeping foreground tracking coherent when browser context fails is not met for the incognito case, because the app identity is dropped. This is a behavior change from the previous code which also blanked app and title for incognito, so it is not a regression, but it is a contradiction with the PR's stated design. The finding is that the fallback path keeps app identity, but the incognito path does not, so the two paths are inconsistent: a user in incognito Chrome gets a heartbeat with app="" which may be filtered or displayed as an empty app, while a user in a normal Chrome window with SB failure gets app="Google Chrome". The observable consequence is that incognito browsing is not tracked as a gap but as an empty-app event, which may be merged or displayed incorrectly. This is a real defect in the new logic because the PR claims to keep foreground tracking coherent, but the incognito branch still drops the app identity. (2/2 passes)