feat(hints): show hints in any Electron app without whitelisting it - #1015
feat(hints): show hints in any Electron app without whitelisting it#1015gabrielecirulli wants to merge 4 commits into
Conversation
Greptile SummaryThis PR broadens accessibility wake-up support for hint mode.
Confidence Score: 4/5The process cache and launch-time accessibility path can leave hints unavailable.
internal/core/infra/electron/electron.go; internal/app/lifecycle.go
 T-Rex LogsWhat T-Rex did
|
| Filename | Overview |
|---|---|
| internal/core/infra/electron/electron.go | Replaces framework-specific enablement with a shared per-PID cache for manual and enhanced AX attributes. |
| internal/app/lifecycle.go | Runs accessibility setup on every focused app while hints are enabled and removes the prior retry loop. |
| internal/core/infra/electron/enable_test.go | Adds coverage for sequential cache, failure, enhanced-gating, and cross-bundle PID-reuse behavior. |
| configs/default-config.toml | Clarifies that enhanced AX support is for browser web content. |
| docs/CONFIGURATION.md | Updates AX support documentation and the window-management caveat. |
| docs/TROUBLESHOOTING.md | Updates Electron and browser hint troubleshooting guidance and log messages. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
internal/core/infra/electron/electron.go:89-91
**Same-bundle PID reuse skips setup**
When a process exits and macOS later reuses its PID for a new instance of the same bundle, this comparison preserves the old `manual` and `enhanced` flags. The attributes are process-local, so the new process skips `AXManualAccessibility` and can keep its Electron accessibility tree asleep, leaving hints unavailable until it receives another PID.
### Issue 2 of 2
internal/app/lifecycle.go:439
**Launch-time AX failure is not retried**
A newly launched Electron or Chromium app can receive its focus event before its AX endpoint is ready. This now makes one lookup/set attempt and returns; unlike the removed backoff loop, it does not retry during that activation, so opening hints immediately after launching a slow app can show no targets until the user focuses the app again.
Reviews (1): Last reviewed commit: "feat(hints): show hints in any Electron ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if !strings.EqualFold(state.bundle, bundleID) { | ||
| state = axState{bundle: bundleID} | ||
| } |
There was a problem hiding this comment.
When a process exits and macOS later reuses its PID for a new instance of the same bundle, this comparison preserves the old manual and enhanced flags. The attributes are process-local, so the new process skips AXManualAccessibility and can keep its Electron accessibility tree asleep, leaving hints unavailable until it receives another PID.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/core/infra/electron/electron.go
Line: 89-91
Comment:
**Same-bundle PID reuse skips setup**
When a process exits and macOS later reuses its PID for a new instance of the same bundle, this comparison preserves the old `manual` and `enhanced` flags. The attributes are process-local, so the new process skips `AXManualAccessibility` and can keep its Electron accessibility tree asleep, leaving hints unavailable until it receives another PID.
How can I resolve this? If you propose a fix, please make it concise.| delay *= backoffFactor | ||
| } | ||
| }() | ||
| go electron.EnsureAppAccessibility(bundleID, useEnhanced, a.logger) |
There was a problem hiding this comment.
A newly launched Electron or Chromium app can receive its focus event before its AX endpoint is ready. This now makes one lookup/set attempt and returns; unlike the removed backoff loop, it does not retry during that activation, so opening hints immediately after launching a slow app can show no targets until the user focuses the app again.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/app/lifecycle.go
Line: 439
Comment:
**Launch-time AX failure is not retried**
A newly launched Electron or Chromium app can receive its focus event before its AX endpoint is ready. This now makes one lookup/set attempt and returns; unlike the removed backoff loop, it does not retry during that activation, so opening hints immediately after launching a slow app can show no targets until the user focuses the app again.
How can I resolve this? If you propose a fix, please make it concise.
Greptile SummaryThis PR unifies accessibility enablement for focused applications. The main changes are:
Confidence Score: 4/5The focused-app accessibility path can miss slow-starting apps, and disabling enhanced support does not clear its active browser state. A one-shot AX setup replaces the prior retry and readiness wait. Enhanced browser accessibility remains active for an existing process after configuration disables it.
T-Rex LogsWhat T-Rex did
|
| Filename | Overview |
|---|---|
| internal/app/lifecycle.go | Runs unified accessibility setup for every activation while hints are enabled. |
| internal/core/infra/electron/electron.go | Replaces per-framework retries with PID-scoped accessibility attribute state. |
| internal/core/infra/electron/enable_test.go | Adds coverage for attribute caching, retries, logging, enhanced gating, and PID reuse. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
internal/app/lifecycle.go:439
**Slow AX startup loses enablement**
When an Electron or Chromium app gains focus before its AX application/tree is ready, this single asynchronous call returns without setting up accessibility. The old path retried with backoff and waited for a usable tree; now hints stay unavailable until the user leaves and refocuses the app.
### Issue 2 of 2
internal/core/infra/electron/electron.go:115
**Enhanced accessibility remains enabled**
After enhanced support is enabled for a browser PID, changing the setting off makes `useEnhanced` false and skips this branch, but no path clears `AXEnhancedUserInterface`. The browser can therefore keep the documented tiling-manager relayout/window-move side effect until it exits even though the configuration now disables enhanced support.
Reviews (2): Last reviewed commit: "feat(hints): show hints in any Electron ..." | Re-trigger Greptile
| delay *= backoffFactor | ||
| } | ||
| }() | ||
| go electron.EnsureAppAccessibility(bundleID, useEnhanced, a.logger) |
There was a problem hiding this comment.
When an Electron or Chromium app gains focus before its AX application/tree is ready, this single asynchronous call returns without setting up accessibility. The old path retried with backoff and waited for a usable tree; now hints stay unavailable until the user leaves and refocuses the app.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/app/lifecycle.go
Line: 439
Comment:
**Slow AX startup loses enablement**
When an Electron or Chromium app gains focus before its AX application/tree is ready, this single asynchronous call returns without setting up accessibility. The old path retried with backoff and waited for a usable tree; now hints stay unavailable until the user leaves and refocuses the app.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| return true | ||
| } | ||
| if useEnhanced && !state.enhanced { |
There was a problem hiding this comment.
After enhanced support is enabled for a browser PID, changing the setting off makes useEnhanced false and skips this branch, but no path clears AXEnhancedUserInterface. The browser can therefore keep the documented tiling-manager relayout/window-move side effect until it exits even though the configuration now disables enhanced support.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/core/infra/electron/electron.go
Line: 115
Comment:
**Enhanced accessibility remains enabled**
After enhanced support is enabled for a browser PID, changing the setting off makes `useEnhanced` false and skips this branch, but no path clears `AXEnhancedUserInterface`. The browser can therefore keep the documented tiling-manager relayout/window-move side effect until it exits even though the configuration now disables enhanced support.
How can I resolve this? If you propose a fix, please make it concise.
|
First, thanks for all the hard work on this matter! Appreciate all the efforts and energy that you brings in to the project. Since this is the first tip of the stacked PRs, I will just comment on this first and ignore the rest of the PRs until this is merged, as it's very scoped and easy to look through and comment on it. I still think that we should keep the gate of I am just thinking if setting it on every app is a good practice or not. Should we look into a way to detect electron bundles or framework and set only on those instead? A project of my friend has some example that seems working nicely, feel free to check it out. https://github.com/blindFS/Glyphlow/blob/a5d7e5ba3f5a61f1641b309e2ee66bf7a05b79e3/src/os_util.rs. By detecting the framework properly, we probably don't have to simply set the attributes to every app, saves an additional AX call and avoid potential side effects on existing native apps too (if it ever will, not sure). And also if this works as intended, we could also remove the Just a side note, even if it's breaking change, don't do |
Hints previously appeared inside an Electron app only if its bundle ID was on a built-in list or added to additional_electron_bundles by hand, because that list decided whether neru set AXManualAccessibility to wake the app's accessibility tree. Unlisted Electron apps showed no hints. neru now sets AXManualAccessibility on every focused app whenever hints are enabled. The attribute wakes Electron and Chromium trees and is a harmless no-op on apps that do not implement it, with no window side effect, so it needs no whitelist and no cross-process tree-walk probe to decide when to apply it. AXEnhancedUserInterface, which exposes Chromium/Firefox web-page content but can move windows under tiling window managers, stays gated. It is set only on Chromium/Firefox browsers, and only when hints.additional_ax_support.enable is on. Electron apps no longer receive it on their own. Changes: - electron.go: replace the three per-family Ensure* functions, the depth-10 tree-walk probe, and the retry loop with one EnsureAppAccessibility that sets manual always and enhanced only when asked. Each successful set is cached per pid (keyed with the bundle id to survive pid reuse); a failed set is retried on a later focus and logged once so a real failure is visible without spamming the log on every focus of an app that does not support the attribute. - lifecycle.go: run the enablement whenever hints are enabled rather than only when the setting is on, and derive the enhanced flag from the Chromium/Firefox lists behind the setting. - enable_test.go: cover the cache, retry-on-failure, log-once, enhanced gating, and pid reuse with a fake attribute setter and a log observer. - electron_test.go: drop the tests for the deleted Electron-only helpers. - docs and default config: Electron works out of the box, the setting now describes browser web-content hints, the troubleshooting log lines match the code, and a tiling-window-manager caveat covers the enhanced attribute. The Electron and Chromium bundle lists stay: tree.go still uses them to prune noisy web DOM trees while scanning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses review feedback on the accessibility enablement. - Retry with exponential backoff when an app is focused before its accessibility tree is ready, so hints appear in a slow-launching app without a manual refocus. The retry lives in EnsureAppAccessibility, and its burst is confined to an app's first encounter, so ordinary native apps (which do not take the attribute) are not re-probed on every focus. - Clear AXEnhancedUserInterface on the next focus of a browser after hints.additional_ax_support is turned off, so its tiling-window side effect does not outlast the setting. - Drop an app's cached accessibility state when it terminates, so a process that later reuses the retired pid has its attributes set again instead of inheriting stale per-process flags. This also bounds cache growth. Tests cover readiness reporting, first-encounter retry gating, the enhanced teardown, and the terminate-time cache reset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
740d9df to
f3d1b87
Compare
neru wakes a focused app's accessibility tree so it can read hint targets. It sets AXManualAccessibility on every focused app, and sets AXEnhancedUserInterface on browsers that need it to expose web-area content. AXEnhancedUserInterface can relayout or move windows under tiling window managers, so it should stay off where it is not needed. Chromium browsers (Chrome, Arc, Brave) expose their web-area hint targets with AXManualAccessibility alone, so this restricts the enhanced attribute to Firefox browsers, which still require it. Chromium browsers now get AXManualAccessibility only. This also removes the electron.ShouldEnableChromiumSupport and electron.IsLikelyChromiumBundle helpers and their tests, which only fed the dropped Chromium branch of the enhanced-attribute gate. The additional_chromium_bundles config field and KnownChromiumBundles stay: the accessibility-tree traversal (isChromiumOrElectron) uses them to find web-area hint targets in Chromium and Electron apps, which is independent of the enhanced attribute. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
I've made a couple relevant findings:
For now the code that sets -- @y3owk1n following up on your comment above:
IMHO, Neru should show hints in browsers and Electron by default and without config. That's the reason why I suggested blindly setting As things currently stand in this PR, that's the only attribute needed (with the exception of Firefox, where
Personally, if Detecting Electron apps would add complexity. Although your friend's project uses a fairly elegant approach, it's still extra maintenance burden. There's always a minor risk that someone somewhere will implement a custom attribute with exactly the same name as
Right now I haven't checked exactly what it changes, but I assume there's a valid reason why those exceptions were made. I do agree we should remove We should ask ourselves the following:
AFAICT there was only one commit in the stack and one PR title with |
Based on my experience working with accessibility (i could be wrong), is that once you set the
in
I think it's still required as they are 2 different things for 2 different platform (maybe I am wrong), but same as electron, attributes that are set doesnt get unset when you restart neru. You have to restart the actual app instead. Note that if you have other apps like homerow for example that is running, it will automatically set these attributes too without neru acknoweledges, and neru will still be able to get the tree without needing to set any attributes.
As far as I remember,
Feel free to try what I said above, quit all apps that will implicitly set those attributes (e.g. homerow, shortcat, hammerspoon. etc), and also quit the chrome or electron apps and restart them, so that these attributes are unset. At this point, you shouldn't get any hints for the web page content at all. And then try to enable those attributes and re-test again. To turn off the attributes, you need to restart the app, not neru. I'll leave the next part uncommented first, if what I said above is true, you might have a different conclusion for the comment follow up section, let me know if you have tested the above. |
That's what I initially suspected too, but during testing I made sure to recompile and rerun Neru and restart each app every time. I was still seeing that behavior. In any case it's not a very useful behavior since it doesn't help on first launch, unless we find a way to force it there without setting the flag.
Hmm, just like with the Electron apps I was restarting the browsers each time. Chrome did seem to keep working whenever I recompiled Neru to only send
When I researched it this morning, it looked like Maybe you should give it a spin as well, commenting out |
|
Sorry, I was probably wrong about Chromium supporting
I'm 99% sure I was restarting the browser each run. I'll need to test again, but I have no good explanation for what I saw, other than maybe some other part of Neru's code setting |
Hey, you're right about this, i actually spin up a test manually with The only thing is that, I am not sure how recent is this tho, if we are all-in into electron + chromium with
I think in our main branch right now, we already have a function to poll on accessibility I quickly glance through your code, seems like you're doing
Forget on what I said previously, as I can confirmed that Again I am sorry that I did not validate what I said properly and just based on what I remember, that's a huge mistake. What we should do next is that:
If all of the above are possible, i would say, go for fully automatic. Remove
I am equally confused now as I came to the same conclusion as what you're describing. And i've checked that only 1 place were setting chromium:
firefox:
another finding: i commented out all of the setting attribute thingy, it seems like as long as manually query the tree a couple of times, and it got activated? I don't know yet. Please allow me some time to verify and do a thorough test... I am confused too... |
|
Some more observations (and updates on what I reported earlier)
Since we've established that nothing in Chromium understands the That would explain why keeping Though it doesn't fully explain why, when we set no flags, the tree does start working only after Cmd+Tab (and not when making repeated attempts at running hints mode before that). Maybe Neru sends something else when the app refocuses? Or something else changes? It couldn't be |
|
I've set Claude on reviewing the Neru codebase side by side with the latest Chromium source to see if it turns up something that explains this. |
|
Disclaimer: written by Claude, after reviewing the Neru source and the current Chromium source. I've read through it but haven't yet written any code to confirm Claude's findings.
However, the code says a second read should return the tree, which does not match what we saw (needs a Cmd+Tab, repeated reads don't do it). No Chromium code reacts to app/window activation, so I can't yet prove where the Cmd+Tab dependency comes from. The section titled "The wake sequence (no flags)" below shows how an application could theoretically turn on Chrome's (and potentially Electron's) accessibility tree solely by means of accessibility queries, wihtout setting attributes on the process. Full technical findings (Neru + Chromium source)What I investigated
|
|
@gabrielecirulli thanks for all the research, I would like to spare some time to actually do some experiment about this locally and come back to you how should we go about next. It's becoming more interesting now. I am testing with minimal changes on a new branch at If this works fine, it's pretty cool, and we can head towards this direction. |
|
Actually, would you like to join our discord channel and discuss over there too? I feel like it's easier to discuss than replying in Gh issues. |
|
In my branch, I am able to get a reliable heuristic checks for bundle type too ("chromium", "electron", "firefox", "webkit"). Tho it will for sure require maintenance effort in the long run, as these are heuristics detection but that's fine. Tested on my side:
Everything seems working fine and smooth with the POC from Summary of the flow that i have here:
UX changes
|
|
Superseded by #1035 |
2 participants
Disclaimer: Both this code and this PR description were written by AI. I still need to review this description to make sure it is fully correct, and I will do that soon.
Draft: this branch also needs review and testing before it moves forward.
Rationale
Hints only appear inside an Electron app if its bundle ID is on a built-in list or added to
additional_electron_bundlesby hand. That list decides whether neru setsAXManualAccessibility, the attribute that wakes an Electron or Chromium accessibility tree. Any unlisted Electron app shows no hints at all.What this changes
neru now sets
AXManualAccessibilityon every focused app while hints are enabled. The attribute wakes Electron and Chromium trees and is a harmless no-op on apps that do not implement it, with no window side effect. So it needs no whitelist and no cross-process tree-walk probe to decide when to apply it.AXEnhancedUserInterfaceexposes Chromium and Firefox web-page content but can shift windows under tiling window managers, so it stays gated. It is set only on Chromium and Firefox browsers, and only whenhints.additional_ax_support.enableis on. Electron apps no longer receive it on their own.Changes
electron.go: replace the three per-familyEnsure*functions, the depth-10 tree-walk probe, and the retry loop with oneEnsureAppAccessibilitythat sets manual always and enhanced only when asked. Each successful set is cached per pid, keyed with the bundle ID to survive pid reuse. A failed set is retried on a later focus and logged once, so a real failure is visible without spamming the log every time an app that lacks the attribute is focused.lifecycle.go: run the enablement whenever hints are enabled, and derive the enhanced flag from the Chromium and Firefox lists behind the setting.enable_test.go: cover the cache, retry-on-failure, log-once, enhanced gating, and pid reuse with a fake attribute setter and a log observer.The Electron and Chromium bundle lists stay.
tree.gostill uses them to prune noisy web DOM trees while scanning.Testing the whole feature
This PR is a prerequisite and does not exercise the feature on its own. To test auto-refresh end to end, pull and run the tip branch
feat/hints-auto-refresh-config(#1018), which stacks all four changes. Its description lists the exact config to set and the steps to follow.Merge order
These four PRs are one series and must merge in order, each only after the one before it lands:
refactor/ax-enablement-manual-on-all— show hints in any Electron app without whitelisting it ← this PRrefactor/rename-web-content-hints— renamehints.additional_ax_supporttohints.web_content_hintsfeat/ax-observer-service— push-based AX observer service (inert)feat/hints-auto-refresh-config— opt-in hints auto-refreshEach targets
main, so until the ones before it merge, its diff also shows their commits.