Mega cleanup: #413, #416, #418, #419 (close #399)

plugin/addons/godot_ai/plugin.gd

@@ -708,6 +708,10 @@ static func _probe_live_server_status(port: int, timeout_ms: int = SERVER_STATUS
 	result["name"] = str(parsed.get("name", ""))
 	result["version"] = _extract_server_version(parsed)
 	result["ws_port"] = int(parsed.get("ws_port", 0))
+	## `package_path` was added in v2.4.4 (#416) so the dock's
+	## "Incompatible server" banner can name the source of a version
+	## skew. Older servers omit it; treat the missing field as "".
+	result["package_path"] = str(parsed.get("package_path", ""))
 	return result
 
 
@@ -1509,7 +1513,10 @@ func start_dev_server() -> void:
 				OS.set_environment("PYTHONPATH", prev_pythonpath)
 
 		if pid > 0:
-			var suffix := " (PYTHONPATH=%s)" % worktree_src if not worktree_src.is_empty() else ""
+			## Match `server_lifecycle.gd::start_server`'s log wording —
+			## "prefix" since we prepended to any pre-existing PYTHONPATH,
+			## not replaced it. See #429 review.
+			var suffix := " (PYTHONPATH prefix=%s)" % worktree_src if not worktree_src.is_empty() else ""
 			print("MCP | started dev server with --reload (PID %d): %s %s%s" % [pid, cmd, " ".join(inner_args), suffix])
 		else:
 			push_warning("MCP | failed to start dev server")

plugin/addons/godot_ai/update_reload_runner.gd

@@ -84,6 +84,16 @@ var _paths_written = []
 ## is missing or stale on disk. Surfaces FAILED_MIXED so the runner refuses
 ## to re-enable the plugin against a half-installed tree.
 var _restore_failed := false
+## Test-only opt-out for the scan-watchdog `push_warning` lines. The
+## watchdog unit tests in `test_update_reload_runner.gd` invoke
+## `_on_scan_watchdog_timeout()` and the post-timeout
+## `_start_filesystem_scan` bypass branch directly to pin their behavior
+## — but those code paths' `push_warning` calls then appear as yellow
+## console noise in every `test_run`, training reviewers to ignore the
+## runner's real production warnings. Tests set this true; production
+## leaves it false so genuine scan timeouts during a real self-update
+## still surface loudly. See issue #413.
+var _suppress_scan_warnings := false
 
 
 func start(zip_path: String, temp_dir: String, detached_dock) -> void:
@@ -166,10 +176,11 @@ func _start_filesystem_scan(next_step: String = "_enable_new_plugin") -> void:
 	## actually completed. Skip the wait; Godot's normal background scan
 	## catches up after the plugin re-enables. See PR #381 review.
 	if _scan_timed_out:
-		push_warning(
-			"MCP | skipping filesystem_changed wait after previous timeout (next_step=%s)"
-			% deferred_step
-		)
+		if not _suppress_scan_warnings:
+			push_warning(
+				"MCP | skipping filesystem_changed wait after previous timeout (next_step=%s)"
+				% deferred_step
+			)
 		call_deferred(deferred_step)
 		return
 
@@ -209,10 +220,11 @@ func _on_scan_watchdog_timeout() -> void:
 	## `_waiting_for_scan == false`.
 	if not _waiting_for_scan:
 		return
-	push_warning(
-		"MCP | filesystem_changed didn't fire within %ds; proceeding without scan confirmation"
-		% int(SCAN_WATCHDOG_SECS)
-	)
+	if not _suppress_scan_warnings:
+		push_warning(
+			"MCP | filesystem_changed didn't fire within %ds; proceeding without scan confirmation"
+			% int(SCAN_WATCHDOG_SECS)
+		)
 	_scan_timed_out = true
 	var fs := EditorInterface.get_resource_filesystem()
 	if fs != null and fs.filesystem_changed.is_connected(_on_filesystem_changed):

plugin/addons/godot_ai/utils/server_lifecycle.gd

@@ -327,17 +327,23 @@ static func _incompatible_server_message(
 ) -> String:
 	var version := _live_version_for_message(live)
 	var actual_ws_port := _live_ws_port_for_message(live)
+	## `package_path` is a v2.4.4+ field — older servers omit it. Suffix
+	## the message with "(loaded from <path>)" when present so the user
+	## can tell *which* `src/godot_ai/` is serving the port without
+	## walking the process tree. See #416.
+	var package_path := _live_package_path_for_message(live)
+	var path_suffix := " (loaded from %s)" % package_path if not package_path.is_empty() else ""
 	if not version.is_empty():
 		if actual_ws_port > 0 and actual_ws_port != expected_ws_port:
 			return (
-				"Port %d is occupied by godot-ai server v%s using WS port %d; "
+				"Port %d is occupied by godot-ai server v%s using WS port %d%s; "
 				+ "plugin expects v%s with WS port %d. Stop the old server or "
 				+ "change both HTTP and WS ports."
-			) % [port, version, actual_ws_port, expected_version, expected_ws_port]
+			) % [port, version, actual_ws_port, path_suffix, expected_version, expected_ws_port]
 		return (
-			"Port %d is occupied by godot-ai server v%s; plugin expects v%s. "
+			"Port %d is occupied by godot-ai server v%s%s; plugin expects v%s. "
 			+ "Stop the old server or change both HTTP and WS ports."
-		) % [port, version, expected_version]
+		) % [port, version, path_suffix, expected_version]
 	var status_code := int(live.get("status_code", 0))
 	if status_code > 0:
 		return (
@@ -366,6 +372,16 @@ static func _live_ws_port_for_message(live: Dictionary) -> int:
 	return int(live.get("ws_port", 0))
 
 
+static func _live_package_path_for_message(live: Dictionary) -> String:
+	## Only trust the path when the live snapshot confirms a godot-ai
+	## server — a probe of some unrelated HTTP service could in theory
+	## return a `package_path` JSON field, and we don't want to mislabel
+	## that as "godot-ai loaded from …" in the incompatible banner.
+	if live.has("name") and str(live.get("name", "")) != "godot-ai":
+		return ""
+	return str(live.get("package_path", ""))
+
+
 # ---- start_server / spawn watch / respawn -----------------------------
 
 ## Branch table (recorded version is the "is this ours?" signal — uvx
@@ -479,8 +495,48 @@ func start_server() -> void:
 		push_warning("MCP | port %d is reserved by Windows (Hyper-V / WSL2 / Docker)" % port)
 		return
 
+	## PYTHONPATH handling for dev checkouts: when the editor is launched
+	## against a worktree whose `src/godot_ai/__version__` differs from the
+	## root repo's editable install, the dev-venv python's `sitecustomize`
+	## adds the *root repo's* `src/` to `sys.path`. The spawned server then
+	## reports the root repo's version, the plugin's compatibility check
+	## flags it as incompatible, and the user gets a Restart-Server loop
+	## with no exit. `start_dev_server` already prepends the worktree's
+	## `src/` for its --reload spawn; mirror that here for the auto-spawn
+	## path so the same worktree-vs-root version skew is impossible. Gated
+	## on `is_dev_checkout()` so production user installs (no nearby `src/`)
+	## are untouched. See #418.
+	var worktree_src := ""
+	var prev_pythonpath := ""
+	var pythonpath_set := false
+	if ClientConfigurator.is_dev_checkout():
+		worktree_src = ClientConfigurator.find_worktree_src_dir(
+			ProjectSettings.globalize_path("res://")
+		)
+		if not worktree_src.is_empty():
+			prev_pythonpath = OS.get_environment("PYTHONPATH")
+			var sep := ";" if OS.get_name() == "Windows" else ":"
+			var new_pp := (
+				worktree_src
+				if prev_pythonpath.is_empty()
+				else worktree_src + sep + prev_pythonpath
+			)
+			OS.set_environment("PYTHONPATH", new_pp)
+			pythonpath_set = true
+
 	_server_pid = OS.create_process(cmd, args)
 	var spawned_pid := int(_server_pid)
+
+	## Restore PYTHONPATH immediately — the spawned child has already
+	## copied the env, so the editor's own process state returns to
+	## baseline. Leaving it set would leak to any later OS.create_process
+	## from unrelated paths.
+	if pythonpath_set:
+		if prev_pythonpath.is_empty():
+			OS.unset_environment("PYTHONPATH")
+		else:
+			OS.set_environment("PYTHONPATH", prev_pythonpath)
+
 	if spawned_pid > 0:
 		_server_spawn_ms = Time.get_ticks_msec()
 		_server_exit_ms = 0
@@ -491,7 +547,13 @@ func start_server() -> void:
 		## editor start's adopt branch heals it to the real port owner.
 		_host._write_managed_server_record(spawned_pid, current_version)
 		_startup_path = McpStartupPathScript.SPAWNED
-		print("MCP | started server (PID %d, v%s): %s %s" % [spawned_pid, current_version, cmd, " ".join(args)])
+		## Log "PYTHONPATH prefix=" rather than "PYTHONPATH=" so the line
+		## isn't misleading when an existing PYTHONPATH was present —
+		## we prepended `worktree_src`, not replaced. Keeps the log
+		## compact (worktree_src is the actionable piece; the full
+		## prev_pythonpath can be 5+ entries long on dev machines).
+		var suffix := " (PYTHONPATH prefix=%s)" % worktree_src if not worktree_src.is_empty() else ""
+		print("MCP | started server (PID %d, v%s): %s %s%s" % [spawned_pid, current_version, cmd, " ".join(args), suffix])
 		_host._start_server_watch()
 	else:
 		set_terminal_diagnosis(McpServerStateScript.CRASHED)

script/local-game-capture-diag

@@ -125,9 +125,21 @@ def _wait_for_session(url: str, sid: str, timeout: float = 15.0) -> dict:
     raise RuntimeError(f"No Godot session registered within {timeout}s. Last response: {last}")
 
 
-def _print_autoload_section(url: str, sid: str) -> bool:
-    """Return True if _mcp_game_helper is present in the autoload list."""
-    found = False
+def _print_autoload_section(url: str, sid: str) -> dict:
+    """Inspect _mcp_game_helper in both the editor's in-memory autoload list
+    and the on-disk [autoload] section of project.godot, and print an
+    actionable HINT keyed to the split state.
+
+    The game subprocess reads project.godot from disk — so in-memory-only
+    presence is a real failure mode (capture never wires up) that's distinct
+    from "missing everywhere" and from "disk-only" (editor needs a restart).
+
+    Returns {"in_memory": bool, "on_disk": bool} so callers can include the
+    split state in downstream FAIL diagnostics rather than collapsing it to
+    a single ambiguous bool.
+    """
+    in_memory = False
+    on_disk = False
     print("\n[1/3] Autoload list (from autoload_manage):")
     try:
         autoloads = _tool_call(url, sid, "autoload_manage", {"op": "list"}, 100)
@@ -141,7 +153,7 @@ def _print_autoload_section(url: str, sid: str) -> bool:
             marker = " <-- target" if name == "_mcp_game_helper" else ""
             print(f"  - {name}: {path}{marker}")
             if name == "_mcp_game_helper":
-                found = True
+                in_memory = True
     except Exception as exc:
         print(f"  autoload_manage failed: {exc}")
 
@@ -168,19 +180,45 @@ def _print_autoload_section(url: str, sid: str) -> bool:
             if in_section:
                 print(f"    {line}")
                 any_lines = True
-                if "_mcp_game_helper" in line:
-                    found = True
+                ## Godot's INI parser treats `;` and `#` as line-comment
+                ## prefixes. Skip those so a commented-out autoload entry
+                ## doesn't count as "found on disk" — its substring would
+                ## otherwise suppress the HINT even though the autoload is
+                ## effectively disabled.
+                stripped = line.lstrip()
+                if stripped.startswith((";", "#")):
+                    continue
+                ## Match on the key only, not the value. The autoload value
+                ## may legitimately point at a path containing the substring
+                ## (unlikely but possible), and matching on `key=` keeps the
+                ## check unambiguous.
+                key = stripped.split("=", 1)[0].strip()
+                if key == "_mcp_game_helper":
+                    on_disk = True
         if not any_lines:
             print("    (no [autoload] section in project.godot)")
     except Exception as exc:
         print(f"  filesystem_manage read_text failed: {exc}")
 
-    if not found:
+    if not in_memory and not on_disk:
         print(
             "\n  HINT: _mcp_game_helper is missing. Disable + re-enable the Godot AI\n"
             "  plugin in Project Settings → Plugins to recreate it."
         )
-    return found
+    elif in_memory and not on_disk:
+        print(
+            "\n  HINT: _mcp_game_helper is in the editor's in-memory ProjectSettings\n"
+            "  but missing from project.godot on disk. The game subprocess reads\n"
+            "  project.godot, so capture will not wire up. Save project (Project →\n"
+            "  Save All / Ctrl-S) to persist the autoload to disk."
+        )
+    elif on_disk and not in_memory:
+        print(
+            "\n  HINT: _mcp_game_helper is on disk but the editor's in-memory\n"
+            "  ProjectSettings doesn't have it. The editor likely needs a reload —\n"
+            "  Project → Reload Current Project, or restart the editor."
+        )
+    return {"in_memory": in_memory, "on_disk": on_disk}
 
 
 def _png_dimensions(png: bytes) -> tuple[int, int] | None:
@@ -286,7 +324,7 @@ def main() -> int:
             print(f"\nFAIL: --session activate '{args.session}' raised: {exc}")
             return 1
 
-    autoload_ok = _print_autoload_section(args.url, sid)
+    autoload_state = _print_autoload_section(args.url, sid)
 
     we_started_run = False
     try:
@@ -323,13 +361,19 @@ def main() -> int:
             print("\nFAIL: game_capture_ready stayed false.")
             print(f"  is_playing={ready.get('is_playing')}")
             print(f"  readiness={ready.get('readiness')}")
-            print(f"  autoload registered={autoload_ok}")
+            ## Surface in-memory vs on-disk separately so the bullet list
+            ## below doesn't contradict itself when only the editor has the
+            ## autoload registered (game subprocess reads disk).
+            print(
+                f"  autoload registered (in-memory={autoload_state['in_memory']},"
+                f" disk={autoload_state['on_disk']})"
+            )
             print(
                 "\n  This is the same condition that produces the 'autoload never\n"
                 "  registered its debugger capture within 20s' timeout. The most\n"
                 "  common causes:"
             )
-            print("    - autoload missing from project.godot (see above)")
+            print("    - autoload missing from project.godot on disk (see above)")
             print("    - game launched outside project_run (manual F5)")
             print("    - game crashed before reaching the mcp:hello beacon")
             _dump_recent_logs(args.url, sid)

src/godot_ai/server.py

@@ -7,12 +7,14 @@
 from collections.abc import AsyncIterator, Iterable
 from contextlib import asynccontextmanager
 from dataclasses import dataclass
+from pathlib import Path
 from typing import Any
 
 from fastmcp import FastMCP
 from starlette.requests import Request
 from starlette.responses import JSONResponse
 
+import godot_ai as _godot_ai_pkg
 from godot_ai import __version__ as _SERVER_VERSION
 from godot_ai.asgi import StaleMcpSessionDiagnosticMiddleware
 from godot_ai.godot_client.client import GodotClient
@@ -56,6 +58,14 @@
 
 logger = logging.getLogger(__name__)
 
+## Filesystem location of the running `godot_ai` package — surfaced via the
+## /godot-ai/status probe so the editor's "Incompatible server" diagnostic
+## can tell the user *which* `src/godot_ai/` was actually loaded. In a
+## multi-worktree dev setup this is the only fast way to distinguish "root
+## .venv resolved to a stale branch" from "wrong PYTHONPATH" without
+## walking the process tree by hand. See issue #416.
+_SERVER_PACKAGE_PATH = str(Path(_godot_ai_pkg.__file__).resolve().parent)
+
 
 @dataclass
 class AppContext:
@@ -221,6 +231,12 @@ async def godot_ai_status(_request: Request) -> JSONResponse:
                 "ws_port": ws_port,
                 "tool_surface": "rollup",
                 "exclude_domains": sorted(exclude),
+                ## `package_path` lets the editor's incompatible-server
+                ## banner pinpoint the source of a version skew (e.g.
+                ## "loaded from /Users/.../godot-ai-feature-branch/src" vs
+                ## "loaded from /Users/.../godot-ai/src") without the
+                ## user having to walk the process tree. See #416.
+                "package_path": _SERVER_PACKAGE_PATH,
             }
         )
 

test_project/tests/test_plugin_lifecycle.gd

@@ -846,6 +846,59 @@ func test_incompatible_server_message_names_ws_port_mismatch() -> void:
 	assert_contains(message, "change both HTTP and WS ports")
 
 
+func test_incompatible_server_message_surfaces_package_path_when_present() -> void:
+	## v2.4.4+ /godot-ai/status carries `package_path` (issue #416). When
+	## the live snapshot includes it, the dock's banner must name the
+	## loaded path so the user can identify a worktree-vs-root version
+	## skew without walking the process tree.
+	var message := GodotAiPlugin._incompatible_server_message(
+		{
+			"name": "godot-ai",
+			"version": "1.4.4",
+			"package_path": "/Users/foo/godot-ai-branch/src/godot_ai",
+		},
+		"2.4.4",
+		18130,
+		McpClientConfigurator.ws_port(),
+	)
+	assert_contains(message, "v1.4.4")
+	assert_contains(
+		message,
+		"(loaded from /Users/foo/godot-ai-branch/src/godot_ai)",
+		"package_path must be surfaced verbatim so the user can match it to a worktree",
+	)
+	assert_contains(message, "plugin expects v2.4.4")
+
+
+func test_incompatible_server_message_omits_path_suffix_when_old_server() -> void:
+	## Old servers (pre-v2.4.4) omit `package_path`. The banner must
+	## degrade gracefully — no trailing "(loaded from )" stub.
+	var message := GodotAiPlugin._incompatible_server_message(
+		{"version": "1.2.10"},
+		"2.2.0",
+		8000,
+		McpClientConfigurator.ws_port(),
+	)
+	assert_false(
+		"loaded from" in message,
+		"missing package_path must not leave an empty parenthetical in the banner",
+	)
+
+
+func test_incompatible_server_message_ignores_package_path_for_non_godot_ai_peer() -> void:
+	## A non-godot-ai server on the port could in theory return a JSON
+	## `package_path` field. Don't label it as "godot-ai loaded from …" —
+	## the surface is for godot-ai version skew, not for misattribution.
+	var message := GodotAiPlugin._incompatible_server_message(
+		{"name": "other-server", "version": "9.9.9", "package_path": "/somewhere/else"},
+		"2.4.4",
+		8000,
+		McpClientConfigurator.ws_port(),
+	)
+	assert_false("loaded from" in message)
+	assert_false("/somewhere/else" in message)
+
+
 func test_incompatible_transition_refreshes_dock_client_statuses() -> void:
 	var plugin := _ProofPlugin.new()
 	var dock := _RefreshDock.new()