Move dock client status reads off main; cache uvx --version#287
Conversation
The dock's `_perform_initial_client_status_refresh` did ~16 sync `FileAccess.open` + `JSON.parse_string` calls on the editor main thread before the loading bar finished, justified as a pre-warm of GDScript's lazy bytecode swap (#233 / #235). The warm only needs to dereference each strategy script once — the 16 disk reads were incidental. Replace with an explicit `_warm_strategy_bytecode` helper that invokes a pure-memory entry point on each of `_json_strategy.gd`, `_toml_strategy.gd`, `_cli_strategy.gd` plus `client_configurator.gd`. No disk, no `OS.execute`. Then route every per-client probe (JSON, TOML, CLI) through the existing worker thread so the dock paints immediately on cold open. Also cache `uvx --version` for the editor session (mirror of the existing `_cached_venv_python` pattern). The dock's `_refresh_setup_status` reads it on the main thread on every dock build and focus-in refresh; ~80 ms cold on Linux, more on Windows. Cache invalidator wired into `_on_install_uv` alongside `McpCliFinder.invalidate("uvx")` so a fresh install is picked up. Trace verification (Linux 4.6.2, free port): Before: dock_attached=397 ms, total=436 ms After: dock_attached=230 ms, total=279 ms (−167 ms, −36 %) Tests: - Updated `test_initial_paint_warms_worker_call_graph_before_threading` to assert the new shape: `_warm_strategy_bytecode()` is called, every config_type is batched into the worker, no sync per-client status probe runs on main, the warm helper stays pure-memory. - Added `test_check_uv_version_caches_for_session` asserting the cache + invalidator + dock install-flow wiring.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR reduces editor main-thread work during dock startup by ensuring client status probes (including JSON/TOML disk reads/parses) run in the existing background worker, and by caching the uvx --version probe for the editor session.
Changes:
- Refactors initial client-status refresh to explicitly warm strategy bytecode once on main, then batch all client probes to the worker thread.
- Introduces a session cache for
uvx --versionwith an explicit invalidator, and wires invalidation into the dock’s uv install flow. - Updates/adds unit tests that lock in the new call graph and caching behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/test_editor_focus_refocus.py | Updates structural assertions for the new warm+worker flow; adds a test covering the uv version cache contract. |
| plugin/addons/godot_ai/mcp_dock.gd | Batches all probe types to the worker; adds _warm_strategy_bytecode; invalidates uv caches after install. |
| plugin/addons/godot_ai/client_configurator.gd | Adds cached check_uv_version() result + invalidation helper to avoid repeated OS.execute calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Drop the cached uvx path AND the cached `uvx --version` so the | ||
| ## next `_refresh_setup_status` finds and reads the freshly-installed | ||
| ## binary instead of returning the pre-install "not found" result. | ||
| McpCliFinder.invalidate("uvx") | ||
| McpClientConfigurator.invalidate_uv_version_cache() |
The original `_on_install_uv` hardcoded `McpCliFinder.invalidate("uvx")`,
but `find_uvx()` caches under `"uvx.exe"` on Windows. After a fresh uv
install on Windows, the CLI-path cache stayed stale and the dock would
keep showing "uv: not found" for the rest of the session.
Add a `McpClientConfigurator.invalidate_uvx_cli_cache()` helper that
routes through the same `_uvx_cli_names()` array `find_uvx()` uses,
so the OS-specific binary name (uvx vs uvx.exe) stays in lockstep
between the populator and the invalidator.
`_on_install_uv` now calls the configurator helper instead of the
finder directly. Tests updated to assert the routing.
Copilot review fix + interactive smoke resultsCopilot review (line 1325) — fixed in
|
| Suite | passed | failed |
|---|---|---|
plugin_lifecycle |
60 | 0 |
dock |
34 | 0 |
dock_dev_server_btn |
4 | 0 |
clients |
77 | 0 |
(Skipped test_run with no suite filter because an unrelated pre-existing flake in test_curve.gd::test_set_points_disk_path_does_not_mutate_cached_resource SIGSEGVs Godot when run as part of the full suite under load — Task 'update_scripts_classes' already exists race in the editor's resource I/O. Reproduces on main without my changes; not introduced by this PR. Worth filing separately.)
editor_reload_plugin (high-risk hot-reload SIGABRT check):
- Reload Add MCP resources and pagination #1: editor PID 21641 stayed alive, plugin reconnected as
test-project@ea90, readinessready. - Reload Support end-to-end reload workflow + codex configurator #2: editor still PID 21641, reconnected as
test-project@15a8, readinessready.
No SIGABRT. The new explicit _warm_strategy_bytecode() helper successfully forces GDScript's lazy bytecode swap before the worker thread spawns — the #235 contract is preserved.
client_manage(op="status") (worker thread populated all 18 clients):
received status for 18 clients
status counts: {'not_configured': 18}
sample: [('claude_code', 'not_configured', True), ('claude_desktop', 'not_configured', False), ...]
All 18 clients (16 JSON + 1 TOML + 1 CLI) returned status. claude_code correctly reports installed: True (CLI path resolved). The worker thread is doing the JSON/TOML/CLI dispatch off-main as designed.
Net for the PR
- Headless trace: dock_attached 397 ms → 230 ms (−42 %)
- Hot-reload safety preserved (2× reload, no SIGABRT)
- All dock-area suites green (175 tests)
- Worker correctly populates statuses for every config_type
- Windows uvx invalidation bug fixed
Generated by Claude Code
Summary
Follow-on to #285 / #286. The remaining startup ditch on the clean spawned path is
dock_attached—_perform_initial_client_status_refreshwas running ~16 syncFileAccess.open+JSON.parse_stringcalls on the editor main thread before the loading bar finished, plus one blockingOS.execute("uvx", ["--version"])on every_refresh_setup_statuscall.This PR moves the disk reads off-main and caches the uvx version probe.
B.1 — Move JSON/TOML status reads off main
The 16 sync disk reads existed as a side-effect of the bytecode pre-warm (#233 / #235): each call dereferences
_json_strategy.gd/_toml_strategy.gd, forcing GDScript's lazy hot-reload bytecode swap to complete before the worker thread enters them. The warm only needs to dereference each strategy script once.New
_warm_strategy_bytecodehelper invokes a single pure-memory entry point on each script:McpJsonStrategy.verify_entry(any_client, {}, "")— pure dict + string compareMcpTomlStrategy.format_body(PackedStringArray(), "")— pure string transformMcpCliStrategy.format_args(PackedStringArray(), "", "")— pure string transformclient_configurator.gdis dereferenced viaclient_ids()/get_by_idNo disk, no
OS.execute, no JSON parse on main. Then every per-client probe (JSON + TOML + CLI alike) is batched into the existing_run_client_status_refresh_workerthread — that worker already dispatches byconfig_type, so this is just removing the artificial main/CLI split.B.2 — Cache
uvx --versionfor the editor sessionMcpClientConfigurator.check_uv_version()ranOS.execute(uvx, ["--version"], ..., true)on every call. Called from_refresh_setup_status(in user mode) on every dock build and focus-in refresh. ~80 ms cold on Linux, more on Windows.Mirror of the existing
_cached_venv_pythonpattern:_uv_version_cache+_uv_version_searchedstatic pair. Invalidatorinvalidate_uv_version_cache()wired into_on_install_uvalongsideMcpCliFinder.invalidate("uvx")so a fresh install is picked up without a session restart.Trace verification
Linux 4.6.2, free port (
path=spawned):That's −167 ms / −36 % on the dock paint phase. On Windows where
FileAccess+JSON.parse_stringcost is typically 2-3× Linux, the win should be larger.Tests
Updated:
test_initial_paint_warms_worker_call_graph_before_threading(tests/unit/test_editor_focus_refocus.py) — asserts the new shape:_warm_strategy_bytecode()is called; every config_type batched into the worker; no sync per-client status probe runs on main; warm helper stays pure-memory (noFileAccess/OS.execute);deferred_cli_probesis gone (the CLI-only stage is dead code now).Added:
test_check_uv_version_caches_for_session— asserts the static cache pair, the short-circuit on_uv_version_searched, the invalidator, and the dock's_on_install_uvcalling both invalidators (CLI path + version) so a fresh install is picked up.Test plan
ruff check src/ tests/cleanpytest -v(692 passed — 691 + 1 new)bash script/ci-check-gdscript— all GDScript files OKtest_runvia MCP in editor — updated + new structural tests pass; existing dock tests unaffectededitor_reload_plugin): verify no SIGABRT — the warm helper preserves Replace timer-based first-refresh deferral with deterministic gate (follow-up to #233/#234) #235's hot-reload safetyGenerated by Claude Code