fix(macos): don't relaunch a browser that quit while frontmost by TimeToBuildBob · Pull Request #152 · ActivityWatch/aw-watcher-window

TimeToBuildBob · 2026-10-02T01:26:35Z

Fixes #151.

Thanks @alexph-dev for the sharp diagnosis. SBApplication(bundleIdentifier:) makes ScriptingBridge launch the app if it isn't running, so when Chrome/Safari quits while frontmost the watcher's follow-up request relaunches it.

Change: address the frontmost process by PID (SBApplication(processIdentifier:)) and skip the request if frontmost.isTerminated, for both the Chrome and Safari paths.

Not built or run: developed on Linux, so unverified on macOS. @alexph-dev, could you check it against your repro (quit Chrome while frontmost)? Based on master; may need a trivial rebase if #146 lands first.

ScriptingBridge addressed by bundle identifier launches the app if it is not running. Target the frontmost process by PID and skip if it already terminated. Fixes ActivityWatch#151. Git-Session-Id: d216

greptile-apps · 2026-10-02T01:28:44Z

Confidence Score: 5/5

[Medium risk] Changes how the window watcher talks to browsers on macOS.

No outstanding blocking issue was identified; the PR appears safe to merge, subject to the macOS verification requested by the author.

Findings

Misleading browser failure log ▶
Browser quit lacks regression coverage ▶

Summary

The PR addresses frontmost Chrome and Safari instances by PID rather than bundle identifier to avoid relaunching a browser that has quit.

The latest change separates termination and proxy-creation failures so they produce distinct logs.

_{Reviews (2) · Last reviewed commit: "fix(macos): distinguish quit from proxy-..."}

greptile-apps · 2026-10-02T01:28:47Z

+      guard !frontmost.isTerminated,
+            let chromeObject = SBApplication.init(processIdentifier: frontmost.processIdentifier) as? ChromeProtocol else {
+        log("Chrome is no longer running, skipping scripting request")
+        return


Misleading browser failure log The guard logs that Chrome is “no longer running” both when it has quit and when ScriptingBridge cannot create a proxy. If proxy creation fails for a running browser, this message hides the reason its URL and title were not captured. Please distinguish those cases; the Safari branch has the same issue.

Fixed in f15ef16 — split the combined guard so the two failure paths log distinct messages:

frontmost.isTerminated → "Chrome/Safari is no longer running, skipping scripting request"

SBApplication(processIdentifier:) proxy creation returning nil → "Failed to create ScriptingBridge proxy for Chrome/Safari"

A still-running browser whose proxy creation fails now logs the real reason instead of "no longer running". No change to the quit-path protection.

greptile-apps · 2026-10-02T01:28:47Z

+      // Address the running process by PID and bail if it already quit: a
+      // bundle-identifier target makes ScriptingBridge relaunch a browser that just quit.
+      guard !frontmost.isTerminated,
+            let chromeObject = SBApplication.init(processIdentifier: frontmost.processIdentifier) as? ChromeProtocol else {


Browser quit lacks regression coverage The new termination guard and PID lookup have not been built or run on macOS, and the test target does not check this behavior. A macOS regression check that quits a frontmost browser would help catch a future change that relaunches it.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Thanks — dismissing as out of scope for this fix. The regression check would need a macOS host driving a real ScriptingBridge browser-quit scenario; our CI does run the suite on macos-latest, but it has no ScriptingBridge/AX harness, and this PR was developed on Linux so it is explicitly marked unverified on macOS pending @alexph-dev's repro (#151).

If the maintainers want it, the follow-up would be adding a unit-level test around a small injectable helper rather than an end-to-end UI test — that is a separate PR, not this one.

TimeToBuildBob · 2026-10-02T01:52:18Z

🤖 AI code review

Safe to merge — no P0/P1 findings on latest review

_{Updated after inline dispositions on finding threads — this is the current state; the verdict below is frozen at review time and is kept as the historical record of that pass.}

Finding disposition

Finding	Severity	State
`aw_watcher_window/macos.swift:612`	P2	rejected

Changes the Chrome and Safari ScriptingBridge paths in aw_watcher_window/macos.swift to address the frontmost process by PID instead of bundle identifier, and adds an isTerminated guard before creating the SBApplication proxy, to avoid relaunching a browser that quit while frontmost.

Needs a look — P2 only

Confidence 4/5

1 finding · ⚠️ 1 P2

⚠️ P2 medium — aw_watcher_window/macos.swift:612

The new guard guard !frontmost.isTerminated is checked after frontmost.bundleIdentifier != nil but the isTerminated property on NSRunningApplication is only meaningful when the process is still running; however, the real issue is that frontmost is captured from NSWorkspace.shared.frontmostApplication at the start of windowTitleChanged, and by the time the ScriptingBridge call is made, the process may have quit. The isTerminated check is a race: it checks the process state at the moment of the guard, but the process could quit between that check and the SBApplication(processIdentifier:) call, which would still relaunch the app. The PR's stated fix is to avoid relaunching a browser that quit while frontmost, but the check does not eliminate the race; it only narrows the window. The observable consequence is that the browser can still be relaunched if it quits in the small window between the isTerminated check and the ScriptingBridge proxy creation, which is the exact bug #151 reports. The fix should use a more robust approach, such as checking isTerminated immediately before the SBApplication call and/or using a process identifier that is verified to be alive, but the current code still has a TOCTOU race.

Remove the `guard frontmost.bundleIdentifier != nil` check since the bundle identifier is no longer used.

_{How this was verified: Checked the surrounding code: frontmost is obtained from NSWorkspace.shared.frontmostApplication at line 589, and the isTerminated check is at line 612, followed by SBApplication.init(processIdentifier:) at line 616. The process can quit between the check and the init call, so the race remains.}

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 — aw_watcher_window/macos.swift

This is a fix(...) PR but no test files are included in the diff. Erik's feedback: 'where is the repro & fixes they are supposed to catch' (gptme#3441), 'that measurement should come with a regression test' (gptme#3446). Add a test that would have caught this bug. (Advisory: Erik merged all such PRs but consistently requested tests.)

Add a test file that reproduces the bug before the fix and passes after it.

_{How this was verified: static preflight: fix-commit + touched-files scan (rule 7)}

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

aw_watcher_window/macos.swift — Replaces SBApplication(bundleIdentifier:) with SBApplication(processIdentifier:) and adds isTerminated guards in both the Chrome and Safari branches.

Previous review passes

commit	score	findings	engine	when
`952759e9ea27`	4/5	1	llm	2026-10-02 01:51 UTC

_{Reviewed f15ef168f17b · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 35s · about this reviewer}

_{Maintainer commands}

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

…uard Greptile flagged that the combined guard logs "no longer running" both when the browser quit and when SBApplication proxy creation fails, hiding the real reason a URL/title was not captured for a still-running browser. Split the guard so each failure path logs its own message. No behavior change to the quit-path protection. Git-Session-Id: d6701f58-58d5-5268-a672-7825ee88d034

TimeToBuildBob · 2026-10-02T02:30:39Z

@greptileai review

TimeToBuildBob · 2026-10-02T03:20:35Z

CI-green and mergeable (Greptile 5/5) — 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.

fix(macos): don't relaunch a browser that quit while frontmost

952759e

ScriptingBridge addressed by bundle identifier launches the app if it is not running. Target the frontmost process by PID and skip if it already terminated. Fixes ActivityWatch#151. Git-Session-Id: d216

TimeToBuildBob mentioned this pull request Oct 2, 2026

macOS: window watcher relaunches a browser that quit while frontmost #151

Closed

greptile-apps Bot reviewed Oct 2, 2026

View reviewed changes

TimeToBuildBob commented Oct 2, 2026

View reviewed changes

Comment thread aw_watcher_window/macos.swift Outdated

ErikBjare merged commit 650c925 into ActivityWatch:master Oct 6, 2026
7 checks passed

This was referenced Oct 6, 2026

fix(macos): keep foreground tracking when browser context fails #146

Open

build(deps): bump aw-watcher-window to f825398 (Gecko browsers for Research Edition + 3 fixes) ActivityWatch/activitywatch#1503

Closed

Navigation Menu

Repository navigation

fix(macos): don't relaunch a browser that quit while frontmost - #152

fix(macos): don't relaunch a browser that quit while frontmost#152
ErikBjare merged 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/macos-no-relaunch-quit-browser

TimeToBuildBob commented Oct 2, 2026

Uh oh!

greptile-apps Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Oct 2, 2026

Uh oh!

TimeToBuildBob Oct 2, 2026

Uh oh!

greptile-apps Bot Oct 2, 2026

Uh oh!

TimeToBuildBob Oct 2, 2026

Uh oh!

TimeToBuildBob commented Oct 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

TimeToBuildBob commented Oct 2, 2026

Uh oh!

TimeToBuildBob commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

TimeToBuildBob commented Oct 2, 2026

Uh oh!

greptile-apps Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Findings

Summary

Uh oh!

greptile-apps Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

TimeToBuildBob Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

TimeToBuildBob Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

TimeToBuildBob commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 AI code review

Safe to merge — no P0/P1 findings on latest review

Needs a look — P2 only

Uh oh!

Uh oh!

TimeToBuildBob commented Oct 2, 2026

Uh oh!

TimeToBuildBob commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Oct 2, 2026 •

edited

Loading

TimeToBuildBob commented Oct 2, 2026 •

edited

Loading