Repository navigation
Conversation
The JXA strategy is the only macOS strategy that can apply privacy_filter
(the swift helper cannot), and its window lookups were unguarded. Safari with
no document, Chrome with no window, or an app whose window attributes are
unavailable threw out of the script, which dropped the poll and left the
heartbeat without a title.
Guard each lookup, read the AX title of the frontmost process as a fallback,
and always emit a title ("No window" when nothing can be read).
Carries ActivityWatch#64 by Michael Jelly, whose diagnosis and approach this follows, with
the review feedback applied: consistent 2-space indentation, one documented
constant instead of repeated comments, and a fallback that calls windows()
instead of indexing the method.
Co-Authored-By: Michael Jelly <53475252+michaeljelly@users.noreply.github.com>
Git-Session-Id: b9165dd9-0d50-48fa-b60e-cbd647875812
[Medium impact] The PR appears safe to merge; both previous findings are fixed and no new actionable issue was found. SummaryKeeps macOS JXA polling alive when the frontmost app has no readable window.
Both previous findings are addressed. The harness passed locally; native macOS behavior remains unverified. Reviews (2) · Last reviewed commit: "fix(macos): discard partial browser meta..." · Reviewed by Greptile |
Git-Session-Id: 3824e6ab-2060-5d77-acd6-13eba49f9e65
Contributor
Author
|
@greptileai review |
Contributor
Author
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Carries #64 (@michaeljelly) onto current master. #64 diagnosed a real bug: the JXA script performs unguarded window lookups, so when the frontmost app has no window we can inspect, the script throws, the poll is dropped, and no heartbeat is sent.
That still matters:
jxais the only macOS strategy that can applyprivacy_filter(the swift helper cannot and the watcher refuses to start), so the JXA path is now load-bearing for privacy-conscious users rather than just a legacy option.What changed
AXTitleas a fallback (works for apps that don't support JXA scripting, and for windows not flaggedAXMain).title. The Python side indexescurrent_window["title"], so a missing key raisedKeyErrorand skipped the heartbeat anyway.NO_WINDOW_TITLEconstant, and a fallback that callswindows()instead of indexing the method (oProcess.windows[0]was undefined).Verification
node --checkon the script body: syntax OK.windows()throws, windows whose attribute access throws, AXMain title, first-window fallback. All 10 pass; every failure path yields a title instead of throwing.poetry run pytest -q: 158 passed.osascript -l JavaScript printAppStatus.jxaon a Mac with an unscriptable app frontmost would close that gap.Attribution
Diagnosis and original approach by @michaeljelly in #64; review notes by @ErikBjare. This PR is the cleaned-up carry.
Ref #64