Merge beta into main: land #415 + #422 self-update fixes (#426)

Author: dsarno

.claude/skills/godot-ai/skill.md

@@ -33,7 +33,7 @@ globs:
   - `client_configurator.gd` — server discovery (venv → uvx → system), client config
   - `mcp_dock.gd` — editor dock panel with status, setup, logs, self-update banner, Tools tab
   - `tool_catalog.gd` — mirror of `src/godot_ai/tools/domains.py`; drives Tools tab; CI-enforced via `tests/unit/test_tool_domains.py`
-  - `update_reload_runner.gd` — self-update extract + plugin re-enable handoff
+  - `update_reload_runner.gd` — self-update single-pass extract, filesystem scan, and plugin re-enable handoff
 - `test_project/` — Godot 4.6 project (plugin symlinked via `addons/godot_ai`, locally built — not tracked in git)
   - `tests/` — GDScript test suites (auto-discovered by test_handler)
 - `tests/` — Python tests (pytest)
@@ -81,13 +81,22 @@ Test guardrails: the runner flags tests with 0 assertions as failures (catches s
 ## GDScript conventions
 
 - Handlers are `@tool` `RefCounted` scripts with **no** `class_name` — load them via `const X := preload("res://addons/godot_ai/handlers/foo_handler.gd")` from `plugin.gd`. The `Mcp*`-prefixed `class_name` is reserved for utility classes shared across the project (e.g. `McpScenePath`, `McpPropertyErrors`, `McpParamValidators`); see #253 for why bare `class_name`s on handlers are forbidden.
+- The `Mcp*` vs preload-only choice is style and namespace hygiene, not a self-update parse-safety mechanism. The fixed runner writes one complete v(N+1) snapshot before the filesystem scan so same-release references see consistent script content.
+- Never delete a `class_name` declaration that has shipped in any release. If a class needs to move or retire, leave the original file path and `class_name` as a compatibility shim. Static constants and static methods usually need explicit forwarding/redeclaration; `extends` alone does not preserve the full lookup shape.
 - Return `{"data": {...}}` on success, `McpErrorCodes.make(code, msg)` on failure — include the failing parameter value and use `error_string(err)` for Godot error codes
 - All scene mutations must use `EditorUndoRedoManager` — response includes `"undoable": true`
 - The dispatcher detects empty/null handler results and reports `INTERNAL_ERROR` — a handler crash no longer looks like success
 - Use `McpScenePath.from_node()` / `McpScenePath.resolve()` for clean paths like `/Main/Camera3D`
 - Use `##` for doc comments, typed arrays (`Array[String]`), never Python-style `"""`
 - Main thread only — 4ms frame budget in `_process()`, use `call_deferred` for mutations
 
+## Self-update compatibility
+
+- `plugin.gd::prepare_for_update_reload()` owns pre-runner server stop prep. `update_manager.gd` owns download, staging, and install gating. `update_reload_runner.gd` owns install, scan, enable, rollback bookkeeping, and detached-dock cleanup after handoff.
+- Forward self-update safety comes from the runner writing `_new_file_paths + _existing_file_paths` in one install pass, then issuing a single `EditorFileSystem.scan()` before re-enable. Do not reintroduce the old new-files scan followed by existing-files scan.
+- Old installed two-phase runners remain in the field until users take their next update. For releases that may be installed by those runners, avoid adding new files that reference constants, methods, or static/non-static shape changes added to existing load-surface scripts in the same release. This applies to both `class_name` scripts and preload-only scripts.
+- For update/reload/extract changes, run `script/local-self-update-smoke` against current source. Historical `--base-from-release-tag` cases document old-runner limits and must not become default CI gates.
+
 ## Python conventions
 
 - Handlers: `return await runtime.send_command("command_name", params)` — don't handle errors
@@ -119,6 +128,8 @@ gh workflow run bump-and-release.yml -f bump=patch   # or minor / major
 ```
 This bumps `plugin.cfg` + `pyproject.toml`, commits, tags, and pushes. The `release.yml` workflow triggers on the tag and builds a `godot-ai-plugin.zip` for the Asset Library. The dock's self-update feature checks GitHub releases on startup and offers one-click updates to users.
 
+Before cutting a release, check the self-update compatibility rules above. In particular, do not delete shipped `class_name` declarations, and keep the release shape friendly to users whose installed runner is still the old two-phase implementation.
+
 ## Dev workflow
 
 - GDScript changes → Reload Plugin in dock

.github/workflows/ci.yml

@@ -150,6 +150,11 @@ jobs:
       - name: Install dependencies
         run: pip install -e ".[dev]"
 
+      - name: Self-update forward upgrade regression
+        shell: bash
+        timeout-minutes: 3
+        run: GODOT_BIN=godot pytest -q tests/integration/test_self_update_upgrade_paths.py
+
       - name: Start MCP server
         shell: bash
         run: bash script/ci-start-server

CLAUDE.md

@@ -24,7 +24,8 @@ AI Client → MCP (stdio/sse/streamable-http) → Python FastMCP server → WebS
 - **Tools return `dict`**: Handlers call `runtime.send_command(command, params)` which returns a dict or raises. Tools create a `DirectRuntime` and delegate to handlers.
 - **Plugin runs on main thread**: All GDScript executes in `_process()` with a 4ms frame budget. Never block. Use `call_deferred` for scene tree mutations.
 - **Scene paths are clean**: `/Main/Camera3D` format, not raw Godot internal paths. Use `McpScenePath.from_node(node, scene_root)` in GDScript.
-- **Class naming**: classes that need a project-wide `class_name` (i.e. used as a type annotation across multiple files) carry the `Mcp*` prefix to avoid colliding with user-project classes. Internals only used inside the plugin (handlers, presets/values, test stubs) skip `class_name` entirely and load via `const X := preload("res://addons/godot_ai/...")` from `plugin.gd` and consumers. Do not add a bare-name `class_name` for a new class — pick `Mcp*` or `preload`.
+- **Class naming**: classes that need a project-wide `class_name` (i.e. used as a type annotation across multiple files) carry the `Mcp*` prefix to avoid colliding with user-project classes. Internals only used inside the plugin (handlers, presets/values, test stubs) skip `class_name` entirely and load via `const X := preload("res://addons/godot_ai/...")` from `plugin.gd` and consumers. Do not add a bare-name `class_name` for a new class — pick `Mcp*` or `preload`. The choice of `Mcp*` vs preload-only is stylistic, not a parse-safety measure; the #398 self-update parse-error class is fixed at the runner by writing one consistent snapshot before scan, and both forms are parse-safe across upgrades from the fixed release onward.
+- **Never delete a published `class_name` declaration**: removing `class_name X` from a class that was registered in any prior released version can trigger a "Could not resolve script" cascade during the self-update disable -> extract -> enable window. This is independent of the runner's single-phase install ordering. If a class_name must be retired, leave the original file path and `class_name` in place as a compatibility shim.
 - **MCP logging**: Plugin prints `MCP | [recv] command(params)` / `MCP | [send] command -> ok` to Godot console. Controlled by `mcp_logging` var.
 - **Tool surface — ~18 named verbs + per-domain `<domain>_manage` rollups**: To stay under hard tool-count caps in clients that ignore Anthropic's `defer_loading` (Antigravity, etc.), each domain exposes one rolled-up MCP tool that takes `op="<verb>"` + a `params` dict, alongside the high-traffic verbs as named tools. Schema-aware clients still see every `op` because `register_manage_tool` in `src/godot_ai/tools/_meta_tool.py` builds a dynamic `Literal[...]` enum. Core tools (`editor_state`, `scene_get_hierarchy`, `node_get_properties`, `session_activate`) stay non-deferred; named non-core verbs and every `<domain>_manage` rollup are tagged `meta={"defer_loading": True}` for tool-search-aware clients. Plugin command names (over WebSocket) are independent — the MCP tool `editor_reload_plugin` dispatches the plugin command `reload_plugin`. See `docs/TOOLS.md` for the full op map.
 - **Tool resources alongside tools**: Read-only `godot://...` URIs mirror the most-used reads (`godot://node/{path}/properties`, `godot://script/{path}`, `godot://materials`, …). Resources don't count against tool caps; tool forms are the fallback for clients that don't surface resources, and the only path that supports per-call `session_id` pinning. When a tool has a resource counterpart, its description appends `Resource form: godot://...` so aware clients can route the cheap reads through the URI.
@@ -33,6 +34,18 @@ AI Client → MCP (stdio/sse/streamable-http) → Python FastMCP server → WebS
 - **Per-call session routing**: every Godot-talking tool accepts an optional `session_id` parameter. Empty (the default) resolves to the global active session. When supplied, that single call targets that session — `require_writable` and every handler inside the call see the pinned session, not the active one. Use this when multiple AI clients share one MCP server. For `<domain>_manage` rollups, `session_id` is a sibling of `op` and `params` (top-level), *not* nested inside `params`. Resources (`godot://...`) still resolve via the active session.
 - **FastMCP middleware order is load-bearing**: `src/godot_ai/server.py` registers, in this order, `PreserveGodotCommandErrorData → StripClientWrapperKwargs → ParseStringifiedParams → HintOpTypoOnManage`. FastMCP composes the chain via `reversed(self.middleware)`, so first-added is **outermost** (sees response last) and last-added is **innermost** (sees response first). The four positions are reasoned out in the docstring above the `mcp.add_middleware(...)` calls in `server.py`; the order is locked by `tests/unit/test_server_middleware_order.py`. Adding new middleware: read that docstring, decide the position, update both the docstring and the test in lockstep.
 
+### Published `class_name` compatibility
+
+Treat a shipped `class_name` as compatibility surface for self-update. v2.4.0 -> v2.4.1 reproduced a 500+ error cascade when `class_name McpErrorCodes` was dropped; v2.4.2 restored it. Single-phase install fixes mixed-snapshot parse errors, but it does not make deleting a previously registered class safe.
+
+If a `class_name` needs to become a shim, keep the original file path and declaration:
+
+- Inheritance-shaped classes can usually `extends "res://addons/godot_ai/.../impl_file.gd"`.
+- Static-constants/static-method classes need explicit forwarding or duplicated constants; `extends` does not surface static members through class-name lookup.
+- Mixed classes should either keep the implementation in the original file or hand-write a shim that preserves every published static and instance shape.
+
+Practical rule: keeping the implementation in the original class_name file is usually simpler and safer than retiring it. If a class truly becomes obsolete, leave a no-op `class_name` stub in place so older projects can pass through the self-update window cleanly.
+
 ## Worktrees
 
 Claude Code sessions often run in git worktrees (`.claude/worktrees/<name>/`). Be aware of which worktree you're in — it affects everything:
@@ -173,6 +186,20 @@ For self-update changes, run the local interactive smoke harness:
 script/local-self-update-smoke
 ```
 
+For runner-ordering changes, the current-as-base form above is the forward
+regression check: it proves the runner shipped in this branch can upgrade to a
+future zip without parse errors. A base from a pre-fix release is a historical
+constraint case: its old installed runner may still print transient parse
+errors during that one upgrade, and PRs in the new version cannot retroactively
+change that runner.
+
+Until old two-phase runners have aged out, release shape matters for the next
+upgrade those users take: avoid adding new files that reference constants,
+methods, or static/non-static shape changes added to existing load-surface
+scripts in the same release. This applies to both `class_name` scripts and
+preload-only scripts because the failure mode is stale Script-object content,
+not just class registry skew.
+
 Agent trigger: this smoke is required whenever a change touches any of these areas:
 
 - `mcp_dock.gd` update check/download/install paths

docs/CONTRIBUTING.md

@@ -76,6 +76,14 @@ script/local-self-update-smoke
 
 It creates a disposable project with a physical `addons/godot_ai/` copy, stages a synthetic v(N+1) plugin ZIP, launches Godot, and prints the single manual action: click Update in the Godot AI dock. After you close Godot normally, the script verifies the fixture version advanced, the update temp dir was consumed, and no new macOS `Godot*.ips` crash report appeared.
 
+### Self-update compatibility rules
+
+Self-update safety depends on the installed runner. Releases that include the fixed runner write one complete v(N+1) snapshot before Godot scans, so future upgrades from that release avoid mixed old/new script parsing. Users on older releases still take their next update through the old two-phase runner, so release shape still matters during that transition.
+
+- Do not delete a `class_name` declaration that has shipped in any release. If a published class needs to move or retire, leave the original file path and `class_name` in place as a compatibility shim.
+- Before cutting a release that may be installed by an old two-phase runner, avoid adding new files that reference constants, methods, or static/non-static shape changes added to existing load-surface scripts in the same release. This applies to `class_name` scripts and preload-only scripts.
+- Keep historical old-runner upgrade tests manual or explicitly marked. Default CI should gate the forward fixed-runner path, not permanently fail on old shipped runner behavior.
+
 ## Dev Server with Auto-Reload
 
 For Python-side changes without restarting Godot:

docs/packaging-distribution.md

@@ -200,5 +200,13 @@ This tier is about install confidence, not deep correctness.
 - [ ] client configuration docs are accurate
 - [ ] CI covers the install surfaces users will actually hit
 - [ ] compatibility guidance exists
+- [ ] no shipped `class_name` declaration was deleted; any retired global class
+      remains at its published file path as a compatibility shim
+- [ ] self-update release shape is compatible with old two-phase runners:
+      new files in `addons/godot_ai/` do not reference constants, methods,
+      or static-ness changes added to existing load-surface scripts in the
+      same release. This applies to both `class_name` scripts and
+      preload-only scripts; old runners fail on stale Script-object content,
+      not just class registry skew.
 - [ ] the first-run experience is clear enough that a new user can succeed without direct help
 

docs/plugin-architecture.md

@@ -1,6 +1,6 @@
 # Godot AI — Plugin Architecture
 
-*Updated 2026-05-04 (add `PreserveGodotCommandErrorData` to the middleware list and note registration-order is load-bearing; previous: refresh file-structure tree, server-side modules, session metadata, and handshake JSON to match shipped code; add `<domain>_manage` rollups + resources + middleware to server responsibilities)*
+*Updated 2026-05-08 (document self-update runner/update-manager/plugin boundary and compatibility rules; previous: add `PreserveGodotCommandErrorData` to the middleware list and note registration-order is load-bearing; refresh file-structure tree, server-side modules, session metadata, and handshake JSON to match shipped code; add `<domain>_manage` rollups + resources + middleware to server responsibilities)*
 
 This document is the architecture reference for the Godot-side plugin and the server-to-plugin interaction model.
 
@@ -71,7 +71,7 @@ plugin/addons/godot_ai/
 ├── mcp_dock.gd                  ## editor dock: status, clients, logs, self-update banner, Tools tab
 ├── client_configurator.gd       ## thin facade for client config (configure/remove/status)
 ├── tool_catalog.gd              ## mirrors src/godot_ai/tools/domains.py; CI-enforced
-├── update_reload_runner.gd      ## self-update extract + plugin re-enable handoff
+├── update_reload_runner.gd      ## self-update single-pass extract, scan, and re-enable handoff
 ├── handlers/                    ## one file per domain; ~30 handlers
 │   ├── editor_handler.gd        ## screenshot, logs, monitors, reload_plugin, quit_editor
 │   ├── scene_handler.gd, node_handler.gd, script_handler.gd
@@ -188,6 +188,23 @@ Outer-to-inner teardown order matters (see #46). Handlers themselves are preload
 
 A symmetric `prepare_for_update_reload()` path runs during self-update so the new plugin version starts (or adopts) the right server.
 
+### Self-update Boundary And Compatibility
+
+The update path is intentionally split so the runner can stay focused on the fragile editor reload window:
+
+- `utils/update_manager.gd` owns pre-runner work: release lookup, download, staging, version checks, and install gating. Its `class_name McpUpdateManager` declaration is published API surface and must remain unless replaced by a same-path compatibility shim.
+- `plugin.gd::prepare_for_update_reload()` owns pre-runner server stop prep. It stops the managed server and resets the spawn guard before the runner starts. Do not move this server lifecycle prep into the runner.
+- `plugin.gd::install_downloaded_update(...)` is the handoff point. It calls `prepare_for_update_reload()`, detaches the dock so it survives plugin teardown, creates the runner, parents it to the editor root, and calls `runner.start(...)`.
+- `update_reload_runner.gd` owns the install-and-reload sequence from that handoff onward: extract files into `addons/godot_ai/`, keep rollback bookkeeping, scan the filesystem, re-enable the plugin, clean up update temp state, and free itself.
+
+The runner's key safety property is a consistent snapshot before scan. It writes all staged new and existing files for v(N+1) in one install pass, then runs one `EditorFileSystem.scan()` before enabling the plugin. This avoids Godot parsing a mixed old/new plugin snapshot and reusing stale Script-object content.
+
+Compatibility rules that follow from that model:
+
+- Never delete a `class_name` declaration that has shipped in a release. Dropping a registered global class can produce a "Could not resolve script" cascade during the disable -> extract -> enable window, independent of the single-pass runner fix.
+- If a published `class_name` has to retire, keep the original file path and declaration as a shape-aware shim. Static constants and static methods need explicit forwarding or redeclaration; simple `extends` is only enough for compatible instance-surface cases.
+- Until old two-phase runners have aged out, release ZIPs should avoid adding new files that reference constants, methods, or static/non-static shape changes added to existing load-surface scripts in the same release. This applies to both `class_name` scripts and preload-only scripts because the failure mode is stale Script-object content, not only class registry skew.
+
 ---
 
 ## Session And Readiness Model

docs/testing-strategy.md

@@ -101,6 +101,8 @@ Passing criteria:
 - the editor process stays alive without manual or programmatic restart
 - the installed fixture plugin version advances to v(N+1)
 - `user://godot_ai_update/` is consumed after install
+- the update window prints no `SCRIPT ERROR: Parse Error`,
+  `ERROR: Failed to load script`, or `Could not resolve script` lines
 - no new `Godot*.ips` appears on macOS
 - the vNext `_exit_tree` trigger does not print during the update window