Fix #398: alias shared classes via non-Mcp* preload consts on parse-hazard load surface

[dsarno]

Summary

The v2.3.2 → v2.4.0 self-update emitted ~30 transient Parse Error lines from five files that referenced new audit-v2 members (McpErrorCodes.MISSING_REQUIRED_PARAM and friends, McpDock._make_header, UpdateReloadRunner.INSTALL_BACKUP_SUFFIX) via bare class_name during the disable→extract→enable window. The new plugin loaded fine afterward, but the affected scripts were unloaded during the window — anything _enter_tree touched in them silently no-oped.

The fix ports the existing connection.gd / dispatcher.gd mitigation pattern into the five files: alias each shared class via a const X := preload(...) local const whose name does not shadow the global class_name, so GDScript routes the lookup through the local-const preload-by-path resolution rather than the registry (which during the parse window holds the cached pre-update class object).

utils/update_mixed_state.gd:20 couldn't take that pattern — the const initializer ran at script-load and would still hit the cache when update_reload_runner.gd's new constants weren't yet visible. Inlined the literal .update_backup instead and added a Python lint that asserts the literal stays in lockstep with the producer's INSTALL_BACKUP_SUFFIX.

Also extends TARGETED_LOAD_SURFACE_FILES to cover the five files so the existing typed-field / constructor / member-access lints prevent the same shape from recurring on this surface. The broader deny-by-default inversion across the whole load surface is tracked in #399.

Closes #398.

Files changed

• plugin/addons/godot_ai/handlers/_node_validator.gd — rename local const aliases McpScenePath/McpErrorCodes → ScenePath/ErrorCodes; add a comment block explaining the parse-hazard rationale that other handlers cite.
• plugin/addons/godot_ai/handlers/animation_presets.gd — add ErrorCodes and ScenePath preload aliases; rename ~70 bare-class_name references.
• plugin/addons/godot_ai/handlers/animation_values.gd — add ErrorCodes and PropertyErrors preload aliases; rename ~20 bare-class_name references.
• plugin/addons/godot_ai/dock_panels/log_viewer.gd — add Dock preload alias for mcp_dock.gd; untype _log_buffer field (type fence stays on the setup(log_buffer: McpLogBuffer) parameter).
• plugin/addons/godot_ai/utils/update_mixed_state.gd — drop the UpdateReloadRunner preload alias; inline BACKUP_SUFFIX := ".update_backup" literal with anti-drift Python regression test.
• tests/unit/test_plugin_self_update_safety.py — extend TARGETED_LOAD_SURFACE_FILES to cover the five files; add test_update_backup_suffix_stays_in_sync.

Test plan

• ruff check src/ tests/ — clean
• ruff format --check src/ tests/ — clean
• pytest -v — 899 passed (includes the new test_update_backup_suffix_stays_in_sync and the extended targeted-load lints over the new files)
• script/ci-check-gdscript — All GDScript files OK
• Live editor smoke — out of reach for this daemon run; CI's Godot tests (Linux/macOS/Windows) covers the GDScript handler tests, and script/local-self-update-smoke is the right next step for end-to-end verification of the install/extract path before a release. Recommended for maintainer before merging since update_mixed_state.gd sits on the install path.

Out of scope (per #399)

• Inverting the lint to deny-by-default across the addon tree.
• Sweeping the other ~30 handlers/*.gd files that still reference shared classes by bare class_name. They aren't currently failing because v2.4.0 didn't add new members behind their references; Self-update parse-hazard lint: drop class_name from heavy internals (McpErrorCodes first) #399 tracks the structurally correct fix.

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR hardens the Godot plugin’s self-update reload window (“disable → extract → enable”) against transient GDScript parse errors caused by parse-time resolution through the global class_name registry. It does so by routing shared-class access through script-local preload(...) aliases (with non-Mcp* names) on the known “reload load-surface”, and by replacing one problematic const alias with an inlined literal plus a Python anti-drift regression test.

Changes:

• Expand the self-update safety lint’s targeted file set and add a regression test that enforces BACKUP_SUFFIX stays identical to INSTALL_BACKUP_SUFFIX without reintroducing the parse hazard.
• Replace bare Mcp* member access in key handler/dock scripts with non-Mcp* const X := preload(...) aliases to avoid registry lookups during the reload window.
• Inline BACKUP_SUFFIX in update_mixed_state.gd (instead of aliasing UpdateReloadRunner.INSTALL_BACKUP_SUFFIX) and guard against drift via unit test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/unit/test_plugin_self_update_safety.py	Extends targeted load-surface lint coverage and adds a suffix anti-drift test.
plugin/addons/godot_ai/utils/update_mixed_state.gd	Inlines backup suffix literal to avoid reload-window parse hazards and documents rationale.
plugin/addons/godot_ai/handlers/animation_values.gd	Adds preload aliases (ErrorCodes, PropertyErrors) and migrates member access off bare Mcp*.
plugin/addons/godot_ai/handlers/animation_presets.gd	Adds preload aliases (ErrorCodes, ScenePath) and migrates member access off bare Mcp*.
plugin/addons/godot_ai/handlers/_node_validator.gd	Renames preload aliases to non-Mcp* names and adds the central parse-hazard rationale comment.
plugin/addons/godot_ai/dock_panels/log_viewer.gd	Uses preload alias for dock helper calls and removes typed field annotation that can trigger parse-time registry lookups.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dsarno]

/review — automated daemon review

Overview

Fixes #398: transient Parse Error lines during self-update from v2.3.2 → v2.4.0 because audit-v2 added new members to shared Mcp* classes referenced via bare class_name from 5 files outside the existing parse-hazard allow-list. Routes those references through script-local non-Mcp* const X := preload(...) aliases so resolution bypasses the cached pre-update class_name registry, and inlines one literal that can't take that pattern (module-level const initializer).

Strengths

• Surgical scope. Only touches the 5 files surfaced by #398. The broader deny-by-default inversion is correctly deferred to #399 — one issue, one PR per CLAUDE.md.
• Self-documenting comments. _node_validator.gd carries the canonical parse-hazard rationale; the other 4 files each have a short preamble pointing back to it. Future readers don't have to git-blame to understand why these aliases exist.
• Anti-drift coverage for the inlined literal. New test_update_backup_suffix_stays_in_sync regex-extracts both INSTALL_BACKUP_SUFFIX (producer) and BACKUP_SUFFIX (consumer) and asserts they match. Replaces the prior runtime-aliasing guard with a build-time one. Docstring explains why the runtime pattern can't be used at script-load on this path.
• Convention alignment. Non-Mcp* local consts (Dock, ScenePath, ErrorCodes, PropertyErrors) intentionally don't shadow the global class_name, which is the entire point of the fix and matches the CLAUDE.md Mcp*-prefix policy in spirit.
• Style consistency. _node_validator.gd migrates from const X = preload(...) → const X := preload(...), matching the codebase's := inference convention.
• Documented type-fence trade-off. _log_buffer going untyped is explicit and the inline comment notes the preserved type-fence on the setup() parameter.
• CI fully green — all 15 checks pass, including Godot tests on Linux/macOS/Windows and game-capture smokes.

Minor observations (non-blocking)

• Drift surface for BACKUP_SUFFIX. The new test pairs update_mixed_state.gd ↔ update_reload_runner.gd only. If a future PR adds a third consumer of the suffix as a literal, it could drift undetected. Acceptable today (single consumer); worth keeping in mind.
• Live self-update smoke not run from the daemon. CLAUDE.md mandates script/local-self-update-smoke for changes that touch the install path, and update_mixed_state.gd is on it. PR description correctly flags this as a recommended step before merge.
• Residual exposure outside the allow-list. A 6th file added later that references shared class members via bare class_name would reproduce the noise on the next release. That's exactly what #399 is scoped to fix.

Tests

• Existing test_targeted_load_scripts_do_not_access_members_via_class_name lint now exercises the 5 newly-listed files.
• New test_update_backup_suffix_stays_in_sync — build-time anti-drift guard.
• 899 Python tests pass; script/ci-check-gdscript clean; Godot suites green on all 3 OSes.

Risk

Low. Pure refactor — runtime semantics unchanged. Member access through a local-const preload is the same dereference cost as through class_name. Renames are mechanical and CI exercises every handler path.

Verdict

LGTM. Recommend the maintainer run script/local-self-update-smoke before merge per CLAUDE.md (PR description already calls this out).

---

Generated by Claude Code

[dsarno]

/simplify — automated daemon cleanup

Three-agent simplify pass on c6d0c96. Aggregated findings, applied as commit 0eccb38:

• Reuse: dropped the 3-line per-file "see _node_validator.gd for the parse-hazard rationale" comments above the new preload aliases in animation_presets.gd, animation_values.gd, and log_viewer.gd. They broke parity with the existing alias sites (connection.gd:24, dispatcher.gd:21) that established this pattern with no per-file rationale. The non-Mcp* naming is the self-documenting signal; readers grep the canonical block in _node_validator.gd.
• Quality: trimmed update_mixed_state.gd's 10-line BACKUP_SUFFIX rationale and the test_update_backup_suffix_stays_in_sync docstring to ~3 lines each — assertion messages already carry the detail. Fixed a stale McpErrorCodes.make(...) mention in _node_validator.gd:69's docstring left over from the rename. Compressed log_viewer.gd's _log_buffer untyped explanation from 4 lines to 3 (kept because the typed-field hazard is distinct from the alias rename).
• Efficiency: no actionable findings. Verified each new preload const is used in its file; no per-call hot-path overhead introduced; update_mixed_state.gd actually drops one script-load preload (UpdateReloadRunner) — net win.

Net diff: −33 / +11 across the 6 files. CI re-running on the new SHA.

Local gates: ruff clean, 899 pytest pass, script/ci-check-gdscript clean.

---

Generated by Claude Code