Close the desktop-automation gaps, and the two failure modes that hid them - #480
Merged
Conversation
Typing, OCR matching, coordinates, the accessibility backend and the
recorder each had a gap that made the public API unusable for real
desktop automation. Each fix below was verified by measurement, not by
inspection.
Typing
- write() looked every character up in a 192-key table and raised on a
miss, so `, . / : ? ! _ + @ %` and all CJK were untypeable. Added
press_unicode / release_unicode / type_unicode_unit (Windows,
KEYEVENTF_UNICODE) plus the plan/supported helpers, and write() now
falls back per character. Newline and tab still send real keys.
OCR
- The engine boxed one word at a time, so "Save As" matched nothing
while plainly on screen. New utils/ocr/text_span.py groups words into
lines by vertical overlap and finds the shortest covering run;
find_text_matches returns the merged box.
Coordinates
- New utils/monitor_layout/logical_frame.py captures and rescales in
logical pixels, so a mixed-DPI, multi-monitor desktop (whose virtual
origin can be negative) reports coordinates a click can use.
Accessibility
- Name matching was exact-only, role="button" never matched on Windows,
max_results truncated before filtering rather than after, and there
was no way to scope a search to one window.
- _find_raw now walks per window and returns at the first match, which
fixes all 19 control-pattern call sites at once instead of threading
window_title through 19 signatures. An unscoped get_state went from
~60s to ~2s; a miss is bounded at ~1.85s; a target at depth 1047 in a
named window resolves in 1.15s. Three separate causes had to be fixed
to get there: per-window iteration, an interruptible ControlViewWalker,
and IUIAutomation2.ConnectionTimeout (one ElementFromHandle against a
full-screen application blocked for 60.04s and was undetectable via
WM_GETOBJECT or IsHungAppWindow).
- Password fields report only {"password": True}; a NULL COM pointer is
not None, so it needs a truthiness check or the walk silently stops
short; COMError is not an OSError subclass and needs catching by name.
- windows_query.py and windows_state.py split out to stay inside the
file-length and complexity limits.
Recorder
- Recordings dropped key releases, had no wheel events and no timestamps,
and leaked a thread per recording. win32_input_hook.py replaces the ad
hoc listeners with one hook pair and a timeline; utils/keyboard_layout
resolves VK to character against the foreground layout.
- utils/input_reach probes whether injected input reaches the system at
all: a foreground application can swallow it while integrity levels
stay identical, so nothing else can tell you the input went nowhere.
url_canon wiring
- utils/url_canon existed and worked but none of its four exits were
connected. Added AC_canonicalize_url / AC_normalize_url / AC_urls_equal
to the executor, the matching ac_* MCP tools, three script-builder
CommandSpecs, and the facade exports.
Test-suite crash
- The headless suite killed the interpreter around 96%, so roughly 500
tests never ran. test_admin_console_thumbnails_gui's fixture called
tab.deleteLater(), but deleteLater is a no-op until an event loop runs
and that module never runs one. Seven AdminConsoleTabs, with the helper
threads they start at construction, survived until the first test that
pumped events, and were destroyed inside a modal dialog's nested exec()
on a worker-driven test. The interpreter died with rc 3221226505
(0xC0000409, a __fastfail): no traceback, and faulthandler cannot see
it. The fixture now flushes the deletion it schedules. The suite runs
end to end for the first time: 4364 passed, 19 skipped.
- This never affected CI, which skips that module for want of the
optional webrtc extra; only developers with it installed hit the crash.
The invariant is recorded in CLAUDE.md.
The url_canon wiring added three executor commands and three MCP tools without updating the documents that count them, which is exactly the drift Progress.md flagged as unguarded. Measured, not adjusted by hand. - Commands 758 -> 761 and MCP tools 664 -> 667, in architecture_explore (summary table, layer diagram, executor rows, mcp_server rows) and in all three READMEs. - utils/ subpackages 306 -> 308 in CLAUDE.md's three references. - utils/executor total 8,811 -> 8,910; action_executor.py 7,918 -> 8,021; utils/accessibility total 2,332 -> 2,390; flow_control.py 758 -> 757 (that one was a line count that merely looked like the command count — left alone by the sweep and corrected separately). - windows_query.py, windows_state.py and win32_input_hook.py were missing from the file-level tables entirely; added, with windows_backend.py's size corrected 773 -> 915. - win32_record.py no longer merges two listeners, so its row said something untrue; rewritten, and the two listener modules are marked as having no remaining callers. - Header scan date and branch refreshed. Progress.md: dropped the two entries that have landed (the interpreter crash, and url_canon's three-sided delivery — its GUI surface is a Script Builder spec, which is what every comparable pure-utility feature has). Added three open items: the six test modules still calling deleteLater without flushing it, the now-unreferenced listener modules, and windows_backend.py sitting at 915 lines against a 750-line limit that no CI job actually enforces. The doc-drift proposal now cites this commit as evidence that it happens in practice. WHATS_NEW/CHANGELOG: record the url_canon wiring, which had shipped in code but appeared in neither.
A downstream caller had abandoned this API and reimplemented window control on pywin32, with a comment explaining why. Both reasons were real bugs, so they are fixed here rather than worked around there. - list_windows handed back ctypes LP_c_long objects instead of integer hwnds. int(hwnd) on one raises ValueError, so a listed window could not be focused, measured or moved — the list was readable and useless. The EnumWindows callback declared its hwnd as POINTER(c_int); it is now HWND. This also un-breaks the ac_list_windows MCP tool, whose handler called int(hwnd) and therefore always raised. - Every Win32 prototype in windows_window_manage now declares argtypes and restype. The default of c_int truncates a 64-bit handle. The module also takes its own user32 handle: prototypes live on the function objects, and utils/window_capture passes its own RECT to GetWindowRect, which sharing would have broken. - close_window_by_title minimised instead of closing, because Win32's CloseWindow() minimises despite its name and the wrapper passed that through — its own docstring said "Minimise". It now posts WM_CLOSE, so the application still gets to run its save prompts; DestroyWindow cannot be used, as it refuses windows owned by another thread. The old behaviour is minimize_window_by_title. Breaking, noted in CHANGELOG. - focus_window restores a minimised window before raising it, but only when it really is minimised — SW_RESTORE un-maximises a maximised one. - show_window_by_title no longer foregrounds a window right after hiding it. New: foreground_window, window_rect, move_window_by_title (omit width/height to reposition without resizing), list_windows(titled_only) and is_window_minimized, across all four surfaces — facade, AC_* command, MCP tool, Script Builder spec. Commands 761 -> 765, tools 667 -> 670, with the architecture map and all three READMEs updated to match. test_window_manage.py covers the parts that were wrong: hwnds are ints, close and minimise reach different backend calls, an omitted size is carried over rather than zeroed, and hiding does not foreground. Progress.md records one thing found but not changed: utils/clipboard has two different get/set_clipboard_image pairs under the same names, one taking bytes and one a path, each with live callers.
Both of these had already happened once and neither was detectable by CI. Queued deleteLater() work is now flushed after every test, by an autouse fixture in test/unit_test/headless/conftest.py. deleteLater() does nothing until an event loop runs and almost no GUI test module here runs one, so a widget — plus any helper thread or timer it started at construction — survived until some later test pumped events and was destroyed inside that unrelated test. Five more modules still had 23 unflushed calls between them; fixing the fixture covers all of them and every test written later, which patching 23 call sites would not. Verified rather than assumed: the explicit flush added earlier to test_admin_console_thumbnails_gui.py is removed in this commit, and the file set that used to kill the interpreter still passes (817 passed), so the fixture is demonstrably what is holding it up. test_doc_counts.py re-measures the command, MCP-tool, utils/ subpackage and examples/ counts and compares them against all 16 places the four documents quote them. CLAUDE.md already required the docs to move with the code and said every count is measured, but nothing enforced it and it drifted: the url_canon wiring left every document quoting the old totals, found later by hand. Checked that the guard actually fires by injecting a wrong number and confirming the failure names the file, both values and what to do. A citation that disappears fails too, rather than passing on a document the pattern can no longer read. Not covered: the GUI tab count, which needs Qt to count _add_tab. Left in Progress.md with the subprocess-probe approach that would work.
The tab count was the one documented figure test_doc_counts.py could not check, because counting tabs means constructing AutoControlGUIWidget and therefore starting Qt. Rather than add a second subprocess for one number, the existing Actions-menu probe — which already builds that widget, in a subprocess, for exactly this reason — now reports it, and the assertion lives alongside it. Confirmed it fires by quoting 99 tabs and checking the failure names the file and both values. All five documented counts are now enforced. .idea was tracked despite .gitignore naming it six different ways: a tracked file ignores .gitignore however many spellings you add, which is presumably why the spellings accumulated. Untracked with `git rm --cached -r` (the files stay on disk), the five redundant rules are replaced by a note pointing at the one rule that does the work, and the working tree stops being permanently dirty with IDE churn.
Three maintainer decisions, applied. Clipboard: utils/clipboard carried two get/set_clipboard_image pairs under identical names — clipboard.py took PNG bytes, clipboard_image.py took a path — each with live callers, so importing the wrong module failed at runtime and only for one of the two argument types. Merged into clipboard.py: set_clipboard_image now accepts bytes or a path, keeping the realpath + isfile validation the path version had, and clipboard_image.py is deleted. Both functions were also unreachable from the package __init__, the facade and execute_action, so they are exported now with AC_clipboard_get_image / AC_clipboard_set_image commands and Script Builder specs to match the MCP tools that already existed. Listeners: windows/listener/ is removed. Win32KeyboardListener and Win32MouseListener had no callers anywhere in the package or the tests once recording moved to win32_input_hook.py, and the package held nothing else. README: the remote-desktop wire protocol is back, in all three languages — HMAC-SHA256 handshake, the shared latest-frame slot that makes a slow viewer drop frames instead of stalling the others, the input allow-list, and the IP allow-list. It was cut in the 1,471-to-267 line rewrite and has no equivalent in examples/ or on the docs site, which is the wrong thing to leave undocumented before someone exposes a host. Every name in the recovered snippets was checked against the current signatures rather than trusted from history. test_clipboard_image.py covers the merge: both argument forms, a non-PNG file re-encoded, empty bytes, a missing file, a directory, wrong types, the round trip, and that the deleted module stays deleted. Commands 765 -> 767; docs and the count guard updated together.
build_default_tool_registry() consults JE_AUTOCONTROL_MCP_READONLY and JE_AUTOCONTROL_MCP_ALIASES. Measuring it with those defaulted meant a developer who had either set in their shell would see the guard fail against documentation that was perfectly correct: with READONLY=1 the registry reports 429 tools where the docs say 670. Measured with both flags pinned instead.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 512 |
| Duplication | 7 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…olders The control-key test asserted that every WRITE_CONTROL_KEYS *name* is in keyboard_keys_table. That is a Windows spelling: Windows calls backspace "back", X11 calls it "backspace" and maps the raw "\b" instead, so the container job failed on Linux. The product code was already correct — write() checks the name against the table before using it and otherwise falls through to the ordinary lookup, which finds "\b" on X11. The test now asserts the property that actually matters, that each control character has *some* key route on this platform, since without one write() would fall through to the space fallback and turn a newline into a space. Checked against both table shapes rather than only the local one. SonarCloud's quality gate failed on new_security_rating with two S5332 (clear-text protocol) hits, both on placeholders I added for AC_urls_equal. Nothing in that example depends on the scheme — it demonstrates that query order and the fragment are ignored — so they are https now rather than suppressed. The AC_canonicalize_url placeholder above keeps http deliberately: it needs port 80 to show default-port removal, and Sonar does not flag it.
|
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
1 participant
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.
Six commits. The work started from a downstream caller that had abandoned
parts of this API and reimplemented them on
pywin32, with commentsexplaining why. Every reason turned out to be a real defect, so they are
fixed here rather than worked around there.
Breaking changes
Three, all in the
Removed/Changedsections ofCHANGELOG.md:close_window_by_title/AC_close_window/ac_close_windownowactually close the window (they post
WM_CLOSE). They previouslyminimised it — Win32's
CloseWindow()minimises despite its name, and thewrapper passed that through, its own docstring saying "Minimise". The old
behaviour is
minimize_window_by_title/AC_minimize_window/ac_minimize_window.je_auto_control.windows.listeneris removed, withWin32KeyboardListenerandWin32MouseListener. Nothing in the package orthe tests referenced them once recording moved to
win32_input_hook.py.je_auto_control.utils.clipboard.clipboard_imageis removed. Itduplicated
clipboard.clipboardunder identical names but a differentset_clipboard_imagesignature. Import fromje_auto_control.utils.clipboardor the facade instead.Fixes
list_windowsreturned unusable handles. TheEnumWindowscallbackdeclared its hwnd as
POINTER(c_int), so every handle arrived as anLP_c_longobject andint(hwnd)raisedValueError. The list could beread and nothing else. This also un-broke the
ac_list_windowsMCP tool,whose handler called
int(hwnd)and therefore always raised. Every Win32prototype in
windows_window_managenow declaresargtypes/restype— thec_intdefault truncates a 64-bit handle — and the module holds its ownuser32handle so those prototypes cannot leak intoutils/window_capture,which passes its own
RECTtoGetWindowRect.focus_windowdid nothing visible on a minimised window. It now restoresfirst, but only when the window really is minimised;
SW_RESTOREwouldun-maximise a maximised one.
show_window_by_titleforegrounded a window right after hiding it.writeraised on the first character outside a 192-entry key table — ona US layout that is
, . / : ? ! _ + @ %and all CJK, so a URL or a Chinesesentence failed as a whole string. It now falls back per character to
KEYEVENTF_UNICODE.Save As),reporting "not found" for text plainly on screen.
physical-vs-logical pixel difference (measured ~116 px here) and by the
virtual-desktop origin on a mixed-DPI desktop.
role="button"nevermatched on Windows,
max_resultstruncated before filtering so a targetpast the cap could never be found, and there was no way to scope a search to
one window. An unscoped
get_statewent from ~60s to ~2s; three separatecauses had to be fixed to get there, one of them a full-screen application
blocking a single
ElementFromHandlefor 60.04s, undetectable viaWM_GETOBJECTorIsHungAppWindow.leaked a thread per recording — the three missing pieces are exactly what
replay needs.
New
utils/ocr/text_span,utils/monitor_layout/logical_frame,utils/keyboard_layout,utils/input_reach,windows/record/win32_input_hook,accessibility
windows_query/windows_state. Window management gainsforeground_window,window_rect,move_window_by_title,list_windows(titled_only=)andis_window_minimized.utils/url_canonandthe clipboard image helpers reach their delivery surfaces — both already
worked but were wired to nothing.
The two failure modes
Neither was visible to CI, and both had already happened.
never ran.
test_admin_console_thumbnails_gui.pycalledtab.deleteLater(),which is a no-op until an event loop runs, and that module never runs one;
seven
AdminConsoleTabs and their helper threads survived untiltest_usb_acl_prompt.pypumped events inside a modal dialog's nestedexec(). rc 3221226505 (0xC0000409, a__fastfail): no traceback, nothingfaulthandler can see. An autouse fixture in
test/unit_test/headless/conftest.pynow flushes queued deletions afterevery test, covering the 23 other unflushed calls too. CI never saw it
because
test_usb_acl_prompt.pyneeds the optionalwebrtcextra, which CIdoes not install.
CLAUDE.mdrequired the docs to move with thecode and said every count is measured, but nothing enforced it.
test_doc_counts.pyand a check intest_actions_menu_gui.pynow re-measurethe command, MCP-tool, subpackage, example and GUI-tab counts and compare
them against all 19 places the four documents quote them.
Both guards were checked by making them fail on purpose, not just by watching
them pass.
Verification
4409 passed, 19 skippedfor the full headless suite — the first time it hasrun end to end. Green under randomised ordering too.
ruffclean,banditexit 0. Window, clipboard and coordinate behaviour was also verified against
the live desktop, not only through mocks.