fix: make the UIA focus watchdog opt-in - #400
Merged
Merged
Conversation
The watchdog no longer earns what it costs. Its only surviving consumer is Tree.on_focus_change, which debounces the event and writes a debug log line; nothing reads the focus state it tracks. The structure-change handling that once maintained tree_state.interactive_nodes -- adding nodes on ChildAdded, removing them on ChildRemoved, rebuilding subtrees on ChildrenInvalidated -- was removed earlier, and _on_property_change went with it. Against a debug log line, running it costs a dedicated STA thread, a long-lived UIA event subscription, COM construction at import time, and exposure to a native access violation in the event pump that kills the whole server with no Python traceback (#332). WINDOWS_MCP_WATCHDOG now has to be set to on/1/true/yes/enabled to start it. Unset leaves it off. The accessibility tree is built on demand for every tool call regardless, so no tool behaviour changes: verified against the real stdio server, which serves all 20 tools either way and runs with 4 fewer threads when the watchdog is off. manifest.json's user_config default flips to false to match, and its description no longer claims the watchdog "keeps the accessibility tree current", which has not been true since the structure-change code was removed. README and CLAUDE.md updated likewise. Refs #332
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.
Refs #332. Behaviour change: the UIA focus watchdog no longer starts by default.
Why
While working #332 I went looking for what the watchdog actually feeds, and the answer is: nothing. Its only consumer in production is
Tree.on_focus_change, and that method in full debounces duplicate events and writes one DEBUG log line._last_focus_eventis read nowhere except its own debounce check, andset_structure_callback/set_property_callbackare called from nowhere at all.Git history explains it. There used to be a
_on_structure_changedoing the real work — mutatingtree_state.interactive_nodes, adding nodes onChildAdded, removing them onChildRemoved, rebuilding subtrees onChildrenInvalidated, updating them onChildrenReordered. That was deleted, and the log-only_on_property_changewent with it. What survives is the delivery mechanism with its payload removed.So the current cost/benefit is a dedicated STA thread, a long-lived UIA event subscription, COM construction at import time (the hazard #398 had to guard), and exposure to the native access violation in
PumpEventsthat kills the entire server with no Python traceback — in exchange for a debug log line.What changed
WINDOWS_MCP_WATCHDOGbecomes opt-in:on,1,true,yesorenabledstarts it, unset or anything else leaves it off.manifest.json'suser_configdefault flips tofalse, and its description no longer claims the watchdog "keeps the accessibility tree current" — untrue since the structure-change code was removed. README and CLAUDE.md updated to match.Anyone relying on the watchdog can still turn it on, and #399's backoff fix means it behaves better if they do.
Verification
The accessibility tree is built on demand for every tool call, so nothing about tool behaviour depends on this. Confirmed against the real stdio server rather than assumed —
initialize+tools/listunder both settings:All 20 tools served either way, clean exit both times; the default genuinely does not start the thread, and the manifest's boolean maps through correctly in both directions.
Full suite: 604 passed.
On #332
This does not "fix" the crash so much as stop shipping the thing that causes it to every user by default, which for a component with no current function is the honest trade. If the tree-cache maintenance is ever restored, the watchdog can be turned back on — ideally on a polling loop rather than native callbacks, so an access violation in UIA cannot take the server down. I'd suggest keeping #332 open until then.