harden(plugin): stop JSON config re-sort, honor UI opt-out over falsey env

plugin/addons/godot_ai/clients/_json_strategy.gd

@@ -24,7 +24,7 @@ static func configure(client: McpClient, server_name: String, server_url: String
 	var existing: Variant = holder.get(server_name, null)
 	holder[server_name] = build_entry(client, server_url, existing)
 
-	if not McpAtomicWrite.write(path, JSON.stringify(config, "\t")):
+	if not McpAtomicWrite.write(path, JSON.stringify(_narrow_integral_numbers(config), "\t", false)):
 		return {"status": "error", "message": "Cannot write to %s" % path}
 	return {"status": "ok", "message": "%s configured (HTTP: %s)" % [client.display_name, server_url]}
 
@@ -60,7 +60,7 @@ static func remove(client: McpClient, server_name: String) -> Dictionary:
 	var holder := _walk_path(config, client.server_key_path)
 	if holder is Dictionary and holder.has(server_name):
 		holder.erase(server_name)
-		if not McpAtomicWrite.write(path, JSON.stringify(config, "\t")):
+		if not McpAtomicWrite.write(path, JSON.stringify(_narrow_integral_numbers(config), "\t", false)):
 			return {"status": "error", "message": "Cannot write to %s" % path}
 	return {"status": "ok", "message": "%s configuration removed" % client.display_name}
 
@@ -236,3 +236,28 @@ static func _walk_path(root: Dictionary, key_path: PackedStringArray) -> Variant
 			return null
 		cur = cur[key]
 	return cur
+
+
+## Godot's JSON.parse turns every JSON number into a float, so a later
+## JSON.stringify re-emits the user's integer fields as "8080.0" — which strict
+## consumers (Go's encoding/json into an int field, etc.) reject, and which
+## needlessly rewrites every number across the user's *other* entries. Re-narrow
+## exactly-representable integral floats back to int so they serialize without
+## the ".0". Walks dicts/arrays in place and returns the (same) value.
+##
+## Integers above 2^53 already lost precision when Godot parsed them to double,
+## so they're left as the float Godot produced rather than faking exactness —
+## byte-perfect preservation would require not parsing the file at all, and such
+## magnitudes don't occur in MCP client configs.
+static func _narrow_integral_numbers(value: Variant) -> Variant:
+	match typeof(value):
+		TYPE_FLOAT:
+			if is_finite(value) and value == floor(value) and absf(value) < 9007199254740992.0:
+				return int(value)
+		TYPE_DICTIONARY:
+			for k in value:
+				value[k] = _narrow_integral_numbers(value[k])
+		TYPE_ARRAY:
+			for i in value.size():
+				value[i] = _narrow_integral_numbers(value[i])
+	return value

plugin/addons/godot_ai/telemetry.gd

@@ -15,9 +15,9 @@
 ##   2. The `godot_ai/telemetry_enabled` EditorSetting — set through the
 ##      MCP dock and persisted between sessions.
 ##
-## When telemetry is disabled, events are never buffered or sent. If an env
-## var is explicitly set to a non-truthy value, telemetry is enabled even if
-## the editor setting is false.
+## When telemetry is disabled, events are never buffered or sent. Only a
+## *truthy* env var force-disables; a falsey or absent env var falls through
+## to the EditorSetting (which defaults to enabled). See McpSettings.telemetry_enabled.
 ##
 ## Buffering: events recorded before the WebSocket is connected go into
 ## a small bounded buffer and flush on the next `record_event` call once

plugin/addons/godot_ai/utils/server_lifecycle.gd

@@ -375,10 +375,13 @@ static func _live_package_path_for_message(live: Dictionary) -> String:
 ## "godot_ai/telemetry_enabled" is set to false. Returns true if the var was
 ## injected so the caller can unset it after spawning.
 func _inject_telemetry_env() -> bool:
-	## Guard: if the user or CI already set an env var (even to "false"), leave
-	## it alone. McpSettings.telemetry_enabled() only reads the EditorSetting
-	## when no env var overrides are present.
-	if OS.has_environment("GODOT_AI_DISABLE_TELEMETRY") or OS.has_environment("DISABLE_TELEMETRY"):
+	## If telemetry is already disabled by a *truthy* env var, leave the env as
+	## the user/CI set it — the post-spawn cleanup unsets what we inject, so
+	## injecting here would strip their own var from the editor process. A
+	## *falsey* value (e.g. DISABLE_TELEMETRY=0) must NOT count as "handled":
+	## fall through so a dock UI opt-out still reaches the spawned server. The
+	## truthy test mirrors McpSettings.telemetry_enabled() and the Python server.
+	if McpSettings.env_truthy("GODOT_AI_DISABLE_TELEMETRY") or McpSettings.env_truthy("DISABLE_TELEMETRY"):
 		return false
 	if not McpSettings.telemetry_enabled():
 		OS.set_environment("GODOT_AI_DISABLE_TELEMETRY", "true")

test_project/tests/test_clients.gd

@@ -820,6 +820,40 @@ func test_json_strategy_preserves_other_servers() -> void:
 	assert_true(parsed["mcpServers"].has("godot-ai"), "Our entry not added")
 
 
+func test_json_strategy_preserves_integer_fields() -> void:
+	## Godot parses every JSON number as a float; a naive round-trip re-emits the
+	## user's integer fields (ports, counts) as "8080.0", which strict consumers
+	## reject and which churns numbers across the user's other entries. The
+	## strategy must re-narrow integral numbers so ints stay ints. (#528 / TC-2)
+	var path := _scratch_dir.path_join("ints.json")
+	var seed := {
+		"mcpServers": {"someone-else": {"url": "http://other/", "port": 8080, "retries": 3}},
+		"numStartups": 47,
+		"weights": [1, 2, 3],
+	}
+	var f := FileAccess.open(path, FileAccess.WRITE)
+	f.store_string(JSON.stringify(seed))
+	f.close()
+
+	var client := _make_test_json_client(path)
+	var result := McpJsonStrategy.configure(client, "godot-ai", "http://127.0.0.1:8000/mcp")
+	assert_eq(result.get("status"), "ok")
+
+	var content_file := FileAccess.open(path, FileAccess.READ)
+	var content := content_file.get_as_text()
+	content_file.close()
+	# Integers must survive as integers — not be floatified to "8080.0".
+	assert_true(content.contains('"port": 8080'), "port int must be present")
+	assert_false(content.contains('"port": 8080.0'), "port must not become 8080.0")
+	assert_false(content.contains('"retries": 3.0'), "retries must not be floatified")
+	assert_false(content.contains('"numStartups": 47.0'), "top-level int must not be floatified")
+	assert_false(content.contains("1.0,") or content.contains("2.0,"), "array ints must not be floatified")
+	# Still valid JSON, other entry preserved, our entry added.
+	var parsed = JSON.parse_string(content)
+	assert_true(parsed["mcpServers"].has("someone-else"))
+	assert_true(parsed["mcpServers"].has("godot-ai"))
+
+
 func test_json_strategy_refuses_to_overwrite_unparseable_file() -> void:
 	## Regression: if the config file exists but we can't parse it (trailing
 	## comma, stray comment, truncated write), `configure()` used to silently

test_project/tests/test_server_lifecycle.gd

@@ -429,3 +429,27 @@ func test_inject_skips_when_disable_telemetry_already_present() -> void:
 	host.free()
 
 	assert_false(injected, "must not inject when DISABLE_TELEMETRY is already in env")
+
+
+func test_inject_when_env_present_but_falsey_and_setting_disabled() -> void:
+	## A falsey env value (e.g. DISABLE_TELEMETRY=0) must NOT suppress a dock UI
+	## opt-out. The Python server parses the env truthily, so a falsey value
+	## leaves the server *enabled* — the plugin has to inject the disable flag
+	## so the opt-out actually reaches the spawned server. The old
+	## has_environment() guard treated any value (even "0") as "handled" and
+	## silently shipped telemetry against the user's UI choice. (#530)
+	_clear_telemetry_env_vars()
+	OS.set_environment(_TENV2, "0")
+	EditorInterface.get_editor_settings().set_setting(
+		McpSettings.SETTING_TELEMETRY_ENABLED, false
+	)
+	var host := _ManagerHostStub.new()
+	var manager := McpServerLifecycleManagerScript.new(host)
+
+	var injected := manager._inject_telemetry_env()
+	var env_present := OS.has_environment(_TENV1)
+	host.free()
+	_clear_telemetry_env_vars()
+
+	assert_true(injected, "a falsey DISABLE_TELEMETRY must not suppress the UI opt-out")
+	assert_true(env_present, "GODOT_AI_DISABLE_TELEMETRY must be injected for the spawn")