refactor(config): rename hints.additional_ax_support to hints.web_content_hints - #1016
refactor(config): rename hints.additional_ax_support to hints.web_content_hints#1016gabrielecirulli wants to merge 4 commits into
Conversation
Greptile SummaryThis PR renames the web-content hints configuration section and updates accessibility activation behavior.
Confidence Score: 4/5The focused-app accessibility startup path can miss transient initialization failures. The changed path performs one AX attribute write and does not wait for the target tree. A focus event during application startup can leave the first hints session without browser or Electron targets. The configuration rename and legacy-key validation paths are otherwise consistent. internal/core/infra/electron/electron.go
What T-Rex did
|
| Filename | Overview |
|---|---|
| internal/config/config.go | Renames the hints configuration struct field and TOML/JSON names. |
| internal/config/service.go | Rejects removed configuration names before unknown TOML fields are ignored. |
| internal/app/lifecycle.go | Routes focused applications through the renamed accessibility configuration. |
| internal/core/infra/electron/electron.go | Uses cached one-shot AX attribute writes but no longer waits for accessibility readiness. |
| internal/core/infra/accessibility/tree.go | Updates bundle-list reads to the renamed configuration field. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
internal/core/infra/electron/electron.go:86
**Startup readiness retry removed**
When an app is focused while its accessibility target is still initializing, the single attribute write can fail and this function returns immediately. The previous path retried and waited for a usable accessibility tree; now the first hint activation can see an empty Electron or browser tree until the user changes focus again.
Reviews (1): Last reviewed commit: "refactor(config)!: rename hints.addition..." | Re-trigger Greptile
| // ensurePIDAccessibility applies and caches the accessibility attributes for a | ||
| // resolved pid. It is separated from the application lookup so the caching and | ||
| // gating rules can be tested without a live accessibility tree. | ||
| func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger *zap.Logger) { |
There was a problem hiding this comment.
Startup readiness retry removed
When an app is focused while its accessibility target is still initializing, the single attribute write can fail and this function returns immediately. The previous path retried and waited for a usable accessibility tree; now the first hint activation can see an empty Electron or browser tree until the user changes focus again.
Artifacts
Repro: generated transient accessibility readiness test
- Contains supporting evidence from the run (text/x-go; charset=utf-8).
Repro: failing transient readiness test output
- Keeps the command output available without making the summary code-heavy.
Repro: production fake-setter test compiler blocker
- Keeps the command output available without making the summary code-heavy.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/core/infra/electron/electron.go
Line: 86
Comment:
**Startup readiness retry removed**
When an app is focused while its accessibility target is still initializing, the single attribute write can fail and this function returns immediately. The previous path retried and waited for a usable accessibility tree; now the first hint activation can see an empty Electron or browser tree until the user changes focus again.
How can I resolve this? If you propose a fix, please make it concise.5f70aa0 to
28d2b40
Compare
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>
…tent_hints The [hints.additional_ax_support] section only controls whether hints reach web-page content inside Chromium and Firefox browsers. Now that Electron apps are woken without any setting, the old name no longer describes what the section does. Renaming it to [hints.web_content_hints] makes the setting self-describing, and its on/off field is renamed from enable to enabled to match the boolean naming used elsewhere in the config (grid.enabled, systray.enabled). The four bundle lists keep their names. Because neru's TOML loader ignores unknown keys, a config still using the old section or the old field name would silently lose its settings. To prevent that, the loader rejects both through the existing configuration-validation error path, the same one that shows the "Configuration Validation Failed" dialog, with a message naming the new key. Changes: - config.go: rename the AdditionalAXSupport type to WebContentHints and its Enable field to Enabled, updating the toml/json tags to web_content_hints/enabled. - config_defaults.go, lifecycle.go, tree.go: update every reader. - service.go: reject both the removed section and the removed enable field during load with a CodeInvalidConfig error that points at the new key. - service_test.go: cover the rejection of the old section and the old field, and a successful load of the new names. - configs and docs: rename the section and field everywhere they appear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
28d2b40 to
0d90304
Compare
|
Superseded by #1035 |
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
The
[hints.additional_ax_support]section only controls whether hints reach web-page content inside Chromium and Firefox browsers. Now that Electron apps are woken without any setting (#1015), the old name no longer describes what the section does.What this changes
Rename the section to
[hints.web_content_hints], which is self-describing, and rename its on/off field fromenabletoenabledto match the boolean naming used elsewhere in the config (grid.enabled,systray.enabled). The four bundle lists keep their names.neru's TOML loader ignores unknown keys, so a config still using the old section or the old field name would silently lose its settings. To prevent that, the loader rejects both through the existing configuration-validation error path, the same one that shows the "Configuration Validation Failed" dialog, with a message naming the new key.
Changes
config.go: rename theAdditionalAXSupporttype toWebContentHintsand itsEnablefield toEnabled, updating the toml/json tags toweb_content_hints/enabled.config_defaults.go,lifecycle.go,tree.go: update every reader.service.go: reject both the removed section and the removedenablefield during load with aCodeInvalidConfigerror that points at the new key.service_test.go: cover the rejection of the old section and the old field, and a successful load of the new names.Breaking change
The
[hints.additional_ax_support]section is renamed to[hints.web_content_hints]and itsenablefield toenabled. A config that still uses either old name fails to load with a validation error telling the user to rename it. The four bundle lists keep their names.Testing this change
The rename and its load-time checks can be verified on this branch alone (build
refactor/rename-web-content-hints); they do not need the rest of the stack.neru config validatechecks a config file without starting the daemon, andneru config reloadapplies it live.New names load. Put the renamed section and field in your config:
neru config validateprintsConfiguration is valid.neru config reload(or a restart) applies it. Withenabled = trueneru setsAXEnhancedUserInterfaceon Chromium and Firefox so hints reach web-page content; withenabled = falsethey do not.Old section name is rejected. Use the pre-rename section:
neru config validatefails, and a liveneru config reloadrejects it the same way and keeps the previous/default config rather than silently dropping the setting:Old field name is rejected. Use the new section but the old
enablefield:Same rejection, pointing at the field:
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 itrefactor/rename-web-content-hints— renamehints.additional_ax_supporttohints.web_content_hints← this PRfeat/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.