Repository navigation
fix(agent): restart into an Input Monitoring grant this process cannot use - #1593
Closed
aniruddhaadak80 wants to merge 3 commits into
Closed
aniruddhaadak80 wants to merge 3 commits into
aniruddhaadak80 wants to merge 3 commits into
Conversation
…t use macOS resolves the Input Monitoring decision for a process when it starts, so a grant made afterwards is applied to the next launch of the identity and not to the running one. The agent already asks for the permission at arming and relaunches when the consent dialog is answered with a grant, but a grant that arrives later -- in System Settings, with no dialog left to answer -- had no path at all: the watcher republished the new state to clients and nothing restarted the agent, so every in-process retry kept failing against a permission that could never take effect. That is the dead end in AprilNEA#1304, where "Try again" and a GUI restart both leave the launchd job untouched. Record what arming learned about the permission and let the watcher turn a grant this process cannot use into a relaunch, reusing the successor the binary watcher already knows how to schedule. A revoke still does not restart the agent: it held the grant already, and relaunching on every revoke would put the agent straight back into the dialog the user dismissed. The decision is a pure function with unit tests, so the contract is checked on every host CI builds rather than only on a machine that can reproduce a TCC grant.
|
This comment has been minimized.
This comment has been minimized.
5 of 13 tasks
…onitoring-grant-relaunch
| } | ||
|
|
||
| /// A malformed row is not evidence that the space behind it is desktop. | ||
| pub(crate) fn hit_test( |
There was a problem hiding this comment.
pointer.rs does not declare this file as a module, and no production code calls hit_test. Rust therefore never compiles the helper or discovers its two tests. Pointer lookup continues to use the existing implementation, while the new tests provide no coverage.
Knowledge Base Used: Global input hooks
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
On macOS the Input Monitoring decision is resolved for a process when it starts, so a grant made afterwards applies to the next launch of the identity and not to the running one. The agent already asks for the permission at arming and relaunches when the consent dialog is answered with a grant, but a grant that arrives later - in System Settings, with no dialog left to answer - had no path at all: the polling watcher republished the new state to clients and nothing restarted the agent. Every in-process retry therefore kept failing against a permission that could never take effect, which is the dead end in #1304 where both "Try again" and a GUI restart leave the
KeepAlivelaunchd job untouched.This records what arming learned about the permission and turns a grant the current process cannot use into a relaunch, reusing the successor the binary watcher already knows how to schedule.
Changes
crates/openlogi-agent-corewatchers/input_monitoring.rs: newAccess(Granted/NeedsSuccessor) and a pureneeds_successor(access, granted_now)decision, plus four unit tests covering the [Bug]: macOS — granting Input Monitoring has no effect until the agent's launchd job is restarted, so "Try again" can never succeed #1304 sequence, the still-denied steady state, a revoke, and a grant that was already in effect. The decision lives next to the watcher that produces the observation, and is pure so the contract is checked on every host CI builds rather than only where a TCC grant can be reproduced.crates/openlogi-agentlifecycle.rs:Runningnow carries the arming outcome;request_input_monitoringreports it asAccessAtArming(InEffect/GrantedToSuccessor/Denied) instead of only deciding whether to relaunch immediately. The Input Monitoring watcher event republishes the state as before and then, when the grant is one this process cannot use, schedules a relaunch and leaves through the normal shutdown path.schedule_input_monitoring_relaunchhas a non-macOS body so the branch is a plainfalsethere rather than dead code.Behaviour deliberately unchanged: a grant answered in the consent dialog still relaunches at arming, and a refusal is still left alone - relaunching into a declined dialog would ask again on every launch. A revoke never restarts the agent either, since that process held the grant already.
No wire types, locales, packaging or CI inputs are touched, so
PROTOCOL_VERSIONand the locale catalogs are unchanged.Testing
No command below was run. This branch was prepared through the GitHub API without a checkout, so nothing was compiled, formatted, linted or executed locally, and no result is being claimed. CI is the first compile of this diff. These are the commands the diff asks for, in the order I would run them:
Focused proof of the new contract:
cargo test -p openlogi-agent-core input_monitoringLocal gate. The changed packages are
openlogi-agent-coreandopenlogi-agent, and the latter is a dependency of most of the application, so the reverse-dependency closure is the whole workspace - the affected-package tier is not the honest choice here and this is the full tier:This diff is almost entirely
#[cfg(target_os = ...)], so macOS-green proves nothing about the other hosts. Those cross-lints are also not run:cargo xtask ci clippy-windows # or: devenv tasks run openlogi:check-windows cargo clippy --target aarch64-unknown-linux-musl \ -p openlogi-hook -p openlogi-inject -p openlogi-hid -p openlogi-hidpp \ -p openlogi-core -p openlogi-agent -p openlogi-agent-core -p openlogi-ipc \ -p openlogi-permissions --all-targets -- -D warningsIn their place the cfg-gated code was hand-audited line by line, and the audit is what shaped the diff: the relaunch condition is evaluated on every host (so the new field is read, and
needs_successoris used, off macOS too), the only macOS-only piece is the free function that schedules the successor, and its non-macOS twin returnsfalse- which keeps theifbody non-empty everywhere and avoidsdead_code, an emptyif, andunused_asyncunder the pedantic table.Accessandneeds_successorarepubin a library crate, so they cannot be reported as dead on any host.Not required by this diff, and not run:
typos,ast-grep,shell,cargo-deny,publish closure,wasm,MSRV,wire_format, locale parity.Not runtime-tested on hardware. No Logitech device, no macOS host, and no TCC grant were involved on my side. macOS TCC and hardware limits worth knowing before verifying by hand:
Access::Grantedis the only state and nothing can call for a relaunch, so the four unit tests are the only executable coverage of the decision.sudo launchctl procinfo $(pgrep -x openlogi-agent) | grep -i responsibleshould name the agent, notOpenLogi.apporTerminal.app.IOHIDRequestAccessblocks the calling thread, so the prompt still runs off the async runtime; that path is unchanged.One question for the maintainer
The fix triggers on the watcher observing the grant, which is the right hook if
IOHIDCheckAccessreports an out-of-band grant to the running process. If macOS instead keeps the pre-grant answer cached for the life of the process - which the report's 18 minutes of unchanged log lines are consistent with, and which no CI job here can settle - then this path stays quiet and the dead end survives. In that case the remaining fix is the other half of the suggestion in #1304: have the GUI's Input Monitoring row and "Try again" ask the agent to restart, which needs an append-only IPC method and aPROTOCOL_VERSIONbump. I left that out deliberately rather than guess which half you want; tell me which and I will do it.On the report's secondary point about the copy naming a row that does not exist: I did not touch it. The permissions skill is explicit that the identity to grant is OpenLogi Agent and that a grant to the GUI does nothing for the agent, so renaming the row in the message to match what one machine happened to list would contradict the documented identity map. Happy to revisit if the skill's guidance has moved.
Suggested guidance changes
Not applied here, per the guidance-maintenance rule - raised for maintainer review as a separate change:
.claude/skills/openlogi-macos-permissions/SKILL.md§5 could state the consequence more sharply: the existing relaunch covers a grant answered in the dialog, and a grant made in System Settings needs the watcher to hand it to a successor. Worth one sentence now that the second path exists.Fixes #1304