Fix read-only audit findings (correctness, fragility, dead code, tests, docs)

[dsarno]

Summary

A read-only, adversarially-verified audit of the codebase surfaced 29 findings. This PR fixes 28 of them (one declined — see below). No behavior-defining conventions changed; these are bug fixes, robustness improvements, dead-code removal, comment corrections, and test-assertion tightening.

Correctness

• theme set_constant / set_font_size now reject non-numeric values instead of silently coercing garbage to 0 (int("abc")==0 never tripped the existing null guard).
• signal_handler type-checks path/signal/target/method before .is_empty(), so a non-string param returns WRONG_TYPE instead of crashing the dispatcher as an opaque "malformed result".
• script find_symbols now detects static func declarations.
• server_lifecycle stop_server no longer re-appends an already-tracked PID (duplicate kill candidates).

Fragility

• material apply_to_node falls back to the in-memory material when the post-save reload returns null (was clearing the slot / crashing get_class()).
• project/session godot:// resources route through safe_payload(_sync) like every other resource module.
• ci-game-capture-smoke cleanup calls project_manage(op="stop") — project_stop is not a named tool, so the game subprocess was never stopped.
• serve-this-worktree guards --port with no value under set -u.
• test_cli_reload no longer leaks GODOT_AI_DEV_* env vars past teardown.

Efficiency

• client_configurator check_uv_version / _find_system_install use bounded McpCliExec instead of blocking OS.execute. Withdrawn — McpCliExec relies on OS.execute_with_pipe, whose capture is unreliable on Godot 4.3 (broke the 4.3 canary). Reverted; client_configurator.gd is unchanged in this PR. (Net: 2 findings declined — this and the _cli_exec pipe-drain.)

Dead code

• Remove unused _resolve_or_create_player and _snapshot_curve, drop the no-op particle alias identity-map, prefix unused scene_root locals with _.

Test quality

• Fix a vacuous readiness-gate assertion (asserted a command name never sent).
• Assert returned values (not just key presence) in logs/resource read tests.
• Assert color components in test_set_param_color_dict.
• Convert a bare return to skip() in test_project.
• Rename misnamed integration "pagination" tests to round-trip (real pagination is covered in test_mcp_tools.py + unit test_pagination.py).

Docs / comments

• Correct stale telemetry milestone/emit-point comment and mark reserved record/milestone types; fix plugin.gd logger-build comment, batch history docstring, tools core-tool docstring; rename debugger _session_id (it is read in the stale-session guard).

Repo hygiene

• Remove orphaned test_telemetry_setting.gd.uid.

Declined (1)

_cli_exec.gd "drain pipes during the poll loop" — the suggested fix is unsafe: _drain_pipe uses FileAccess.get_as_text(), which reads to EOF and would block until the process exits, defeating the timeout. The bounded-output assumption is already documented at lines 18–20, so the code is left as-is.

Verification

• ruff check src/ tests/ — clean
• pytest — 1096 passed, 4 skipped
• GDScript suite via test_run — 0 failures on Godot 4.6.3 (1366 passed) and 0 failures on Godot 4.3 (1332 passed); 0 parse errors on both engines

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/godot_ai/resources/__init__.py	71.42%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[dsarno]

Follow-up (commit d092594): reverted finding #17.

The first push failed the Godot tests / Linux (Godot 4.3) canary on test_uvx_server_command_uses_exact_pin_not_tilde ("0 assertions"). Root cause was the finding #17 change itself: routing check_uv_version/_find_system_install through McpCliExec uses OS.execute_with_pipe, whose output capture differs on Godot 4.3 (the documented reason the cli_exec capture tests are skipped on < 4.4). On the canary, which godot-ai captured empty → get_server_command() resolved to nothing → the test's assert loop never ran. 4.6 captured fine, so only the 4.3 job failed (and my local 4.3 smoke resolved via dev-venv first, masking it).

Finding #17 was a low-severity defensive tweak, so I reverted it rather than break the minimum-supported engine; client_configurator.gd is now byte-identical to its pre-audit baseline. The Efficiency item in the description is therefore withdrawn — net 27 findings applied, 2 declined.

[Copilot]

Pull request overview

This PR addresses a broad set of audit findings across the Godot plugin, Python MCP server/resources, scripts, tests, and documentation comments. It mainly improves validation, resource error handling, cleanup reliability, and removes unused code.

Changes:

• Tightens validation/error handling for theme values, signal params, resource wrappers, material reload fallback, and server PID cleanup.
• Removes unused helpers/aliases and updates stale comments/docstrings.
• Strengthens or renames tests and fixes script/test cleanup behavior.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/unit/test_runtime_handlers.py	Tightens assertions for logs and audio readiness gate.
tests/unit/test_cli_reload.py	Prevents dev reload env-var leakage across tests.
tests/integration/test_pagination.py	Reframes tests as full-result round-trip coverage.
tests/integration/test_mcp_tools.py	Tightens logs resource assertion.
test_project/tests/test_telemetry_setting.gd.uid	Removes orphaned UID file.
test_project/tests/test_project.gd	Marks unavailable editor setting path as skipped.
test_project/tests/test_material.gd	Adds color component assertions.
src/godot_ai/tools/__init__.py	Corrects core tool documentation.
src/godot_ai/telemetry.py	Corrects reserved telemetry/milestone comments.
src/godot_ai/resources/sessions.py	Wraps sync session resource in safe payload handling.
src/godot_ai/resources/project.py	Wraps project resources in safe payload handling.
src/godot_ai/resources/__init__.py	Adds safe_payload_sync.
script/serve-this-worktree	Handles missing --port value safely.
script/ci-game-capture-smoke	Uses project_manage(op="stop") for cleanup.
plugin/addons/godot_ai/utils/server_lifecycle.gd	Avoids duplicate PID kill candidates.
plugin/addons/godot_ai/plugin.gd	Clarifies logger loading comment.
plugin/addons/godot_ai/handlers/theme_handler.gd	Validates numeric theme scalar inputs before coercion.
plugin/addons/godot_ai/handlers/texture_handler.gd	Marks unused scene root local.
plugin/addons/godot_ai/handlers/signal_handler.gd	Type-checks signal params before string operations.
plugin/addons/godot_ai/handlers/script_handler.gd	Handles static func symbol detection and unused locals.
plugin/addons/godot_ai/handlers/resource_handler.gd	Marks unused scene root locals.
plugin/addons/godot_ai/handlers/particle_handler.gd	Removes no-op CPU particle alias map.
plugin/addons/godot_ai/handlers/material_handler.gd	Falls back to in-memory material on reload failure and marks unused locals.
plugin/addons/godot_ai/handlers/environment_handler.gd	Marks unused scene root local.
plugin/addons/godot_ai/handlers/curve_handler.gd	Removes unused curve snapshot helper.
plugin/addons/godot_ai/handlers/batch_handler.gd	Corrects rollback history comment.
plugin/addons/godot_ai/handlers/animation_handler.gd	Removes unused player auto-create helper.
plugin/addons/godot_ai/debugger/mcp_debugger_plugin.gd	Renames used debugger session parameter.

---

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