From e2491bc4e1a7f8593041ed925550884e22d8fedc Mon Sep 17 00:00:00 2001 From: yvmou_ Date: Fri, 23 Oct 2026 16:56:35 +0800 Subject: [PATCH] =?UTF-8?q?godot=20ai=20=E6=9B=B4=E6=96=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- addons/godot_ai/client_configurator.gd | 61 +++ addons/godot_ai/clients/_base.gd | 181 ++++++++- addons/godot_ai/clients/_cli_finder.gd | 98 ++++- addons/godot_ai/clients/_cli_strategy.gd | 33 ++ addons/godot_ai/clients/_dsh_strategy.gd | 8 + addons/godot_ai/clients/_json_strategy.gd | 29 ++ addons/godot_ai/clients/_path_template.gd | 208 +++++++++- addons/godot_ai/clients/_toml_strategy.gd | 8 + addons/godot_ai/clients/_yaml_strategy.gd | 8 + addons/godot_ai/clients/cline.gd | 8 + addons/godot_ai/clients/kilo_code.gd | 8 + addons/godot_ai/clients/roo_code.gd | 8 + addons/godot_ai/clients/vscode.gd | 9 + addons/godot_ai/clients/zed.gd | 10 + addons/godot_ai/clients/zoo_code.gd | 8 + .../godot_ai/handlers/filesystem_mutation.gd | 310 ++++++++++++-- addons/godot_ai/handlers/node_handler.gd | 103 ++++- addons/godot_ai/mcp_dock.gd | 28 +- addons/godot_ai/plugin.cfg | 2 +- addons/godot_ai/runtime/game_helper.gd | 31 +- addons/godot_ai/runtime/game_logger.gd | 26 ++ addons/godot_ai/utils/client_mutation_lock.gd | 8 + addons/godot_ai/utils/error_codes.gd | 5 + addons/godot_ai/utils/port_resolver.gd | 39 ++ addons/godot_ai/utils/server_lifecycle.gd | 308 ++++++++++++-- addons/godot_ai/utils/transport_capability.gd | 382 +++++++++++++++++- 26 files changed, 1800 insertions(+), 127 deletions(-) diff --git a/addons/godot_ai/client_configurator.gd b/addons/godot_ai/client_configurator.gd index ccfb3b2..1230439 100644 --- a/addons/godot_ai/client_configurator.gd +++ b/addons/godot_ai/client_configurator.gd @@ -613,6 +613,12 @@ static func configure(id: String, url: String = "", launch_context: Dictionary = if not client.automatic_config_edits: return _manual_edit_result(client, "configure") var path_error := _config_path_resolution_error(client) + if path_error.is_empty(): + path_error = _config_write_error(client, "configure") + if path_error.is_empty(): + path_error = _config_create_error(client) + if path_error.is_empty(): + path_error = _credentials_error(client) if not path_error.is_empty(): return {"status": "error", "message": path_error} ## Capture `url` once so a port flip in EditorSettings between write and @@ -857,6 +863,8 @@ static func remove(id: String, url: String = "", launch_context: Dictionary = {} if not client.automatic_config_edits: return _manual_edit_result(client, "remove") var path_error := _config_path_resolution_error(client) + if path_error.is_empty(): + path_error = _config_write_error(client, "remove") if not path_error.is_empty(): return {"status": "error", "message": path_error} if url.is_empty(): @@ -897,6 +905,49 @@ static func _config_path_resolution_error(client: Client) -> String: return str(client.resolved_config_path_details().get("error", "")) +## Why Configure must not create this client's file although status may still +## read the path: `create_error` in `McpClient`'s candidate rules. Kept apart +## from the function above, whose signature `script/local-self-update-smoke` +## patches by exact match, in release trees as well as this one. +static func _config_create_error(client: Client) -> String: + if client.config_type == "cli": + return "" + return str(client.resolved_config_path_details().get("create_error", "")) + + +## Why neither Configure nor Remove may touch this client's file: this editor's +## Flatpak sandbox does not share it with the host (`McpClient.config_write_error`, +## or the CLI strategy's own rule for a client that writes through its CLI, where +## Configure and Remove differ). Every strategy refuses on it too; checking here +## puts the refusal ahead of launcher discovery and the lock. +static func _config_write_error(client: Client, action: String) -> String: + if client.config_type == "cli": + if action == "configure": + return CliStrategy.configure_write_error(client) + return CliStrategy.remove_write_error(client) + return client.config_write_error() + + +## Why Configure must not write an attach entry although the client's file is +## writable: this editor's Flatpak sandbox keeps Godot AI's credentials where +## the bridge that entry launches does not look. A missing entry is better +## than one known to be broken. Remove is not held to it. +static func _credentials_error(client: Client) -> String: + if client.command_shape == Client.CommandShape.NONE: + return "" + if not McpPathTemplate.flatpak_hides_credentials(): + return "" + var app_id := McpPathTemplate.flatpak_app_id() + return ( + "Godot runs in a Flatpak sandbox that keeps Godot AI's credentials to " + + "itself, so the bridge %s starts could not connect and nothing was " + + "changed. Run `flatpak override --user " + + "--filesystem=xdg-config/godot-ai:create %s`, restart Godot, and try " + + "again. docs/steam-capability-directory.md covers the other ways to " + + "share them." + ) % [client.display_name, app_id if not app_id.is_empty() else ""] + + # --- Strategy dispatch + verify (testable seam) -------------------------- static func _manual_edit_result(client: Client, action: String) -> Dictionary: @@ -1240,6 +1291,16 @@ static func manual_command(id: String) -> String: ) if cmd.is_empty(): return cmd + ## The path above is the one outside Flatpak. When this sandboxed editor + ## cannot see where a Flatpak build keeps the same file, that may be the + ## wrong one for this user, and this text is all a refused Configure (or a + ## manual-only client) leaves them to go on. + var hidden_paths: Variant = path_resolution.get("hidden_paths", null) + if hidden_paths is PackedStringArray and not hidden_paths.is_empty(): + cmd += ( + "\n\nA Flatpak build keeps this file at %s instead. Godot's own Flatpak " + + "sandbox cannot see it, so edit that one if it is the build you use." + ) % " or ".join(hidden_paths) ## #507: when the allow-host opt-in names a non-loopback range, also ## surface the LAN URL so the user can copy-paste the right address into ## a remote agent. Informational only — configure/remove still WRITE the diff --git a/addons/godot_ai/clients/_base.gd b/addons/godot_ai/clients/_base.gd index ab27fe0..754ce77 100644 --- a/addons/godot_ai/clients/_base.gd +++ b/addons/godot_ai/clients/_base.gd @@ -79,10 +79,21 @@ var path_template: Dictionary = {} ## a fallback path that may become invisible after copy-on-write. When ## that private leaf is new, Configure seeds it from the first later ## existing candidate so read-through content is not shadowed. -## 2. If no file or wildcard package match exists, the first non-wildcard -## template is the deterministic create target. +## 2. If no file or wildcard package match exists, the create target is the +## non-wildcard template whose own directories already exist, because the +## client has run from there (`_evident_create_path` gives the order), +## and otherwise the first non-wildcard template. This is what sends a +## Flatpak-only IDE's entry to `~/.var/app//config/...` instead of a +## `~/.config` it never reads. ## 3. Multiple matches within any wildcard group are ambiguous and fail ## closed instead of choosing an arbitrary package. +## 4. A template inside another Flatpak app's `~/.var/app/` is never a +## create target while this editor's own Flatpak sandbox hides that +## directory (`McpPathTemplate.hidden_flatpak_app_dir`). When rule 2 then +## finds no existing directory among the rest, the resolution carries +## `create_error` and the unseen `hidden_paths`: status still reads the +## first template, and Configure refuses rather than create a file the +## client may never read. ## ## Exact-file and config-home environment overrides still have higher ## priority. When this map has no entry for the current platform, @@ -331,6 +342,57 @@ func resolved_config_path_details() -> Dictionary: return {"path": "", "error": unresolved_config_path_error(display_name, path)} +## Why Configure and Remove must leave this client's resolved file alone; "" +## when they may write it. See `unshared_flatpak_config_error`. +func config_write_error() -> String: + var path := resolved_config_path() + return "" if path.is_empty() else unshared_flatpak_config_error(display_name, path) + + +## Why Configure and Remove must leave the file at `path` alone when this +## editor runs in a Flatpak sandbox that does not share it read-write with the +## host; "" when it does, and outside Flatpak. A file written there is read +## back and verified by this editor and never seen by the client. +## +## Every strategy asks this before it writes, ahead of `create_error`: where a +## Flatpak build of the client keeps its settings is beside the point until +## the sandbox can write the ordinary location at all. Status does not ask. +## It keeps reading the path, and under a read-only grant that is the +## client's real file. +static func unshared_flatpak_config_error(client_name: String, path: String) -> String: + var block := McpPathTemplate.flatpak_write_block(path) + if block.is_empty(): + return "" + var refused := ( + "Godot runs in a Flatpak sandbox that cannot write %s where %s reads " + + "it, so nothing was changed." + ) % [path, client_name] + var by_hand := "or edit that file by hand (docs/steam-capability-directory.md)." + if block.has("unmounted"): + return refused + ( + " Flatpak was asked to share %s but mounted nothing there, which " + + "is what it does when the directory does not exist as Godot " + + "starts. Create it, restart Godot, and try again, %s" + ) % [block["unmounted"], by_hand] + if block.has("hidden"): + return refused + ( + " Flatpak hides %s from this sandbox, as a --nofilesystem rule " + + "does. Remove that rule, restart Godot, and try again, %s" + ) % [block["hidden"], by_hand] + if block.has("read_only"): + return refused + ( + " %s is mounted read-only. Make it writable, restart Godot, and " + + "try again, %s" + ) % [block["read_only"], by_hand] + var app_id := McpPathTemplate.flatpak_app_id() + return refused + ( + " Run `flatpak override --user --filesystem=%s %s`, restart Godot, " + + "and try again, or edit that file by hand: " + + "docs/steam-capability-directory.md covers a sandbox without that " + + "grant." + ) % [block["needs"], app_id if not app_id.is_empty() else ""] + + ## Shared wording for a path template `McpPathTemplate.expand` could not fully ## resolve. Used by the guard above and by the merge-tier loader in ## `_json_strategy.gd`, which resolves its own templates and never passes @@ -404,7 +466,11 @@ func _resolve_ordered_config_path_candidates(templates: Variant) -> Dictionary: var ordered_templates: Array = [] for template_variant in templates: ordered_templates.append(str(template_variant)) - var fallback_create_path := "" + ## Candidates whose file does not exist yet and that Configure could create. + var create_templates := PackedStringArray() + var create_paths := PackedStringArray() + var hidden_app_dirs := PackedStringArray() + var hidden_paths := PackedStringArray() for index in range(ordered_templates.size()): var template := str(ordered_templates[index]) var expanded_template := McpPathTemplate.expand(template) @@ -444,10 +510,113 @@ func _resolve_ordered_config_path_candidates(templates: Variant) -> Dictionary: return {"path": "", "error": seed["error"]} _clear_config_path_warning() return {"path": path, "error": "", "seed_path": seed.get("path", "")} - if fallback_create_path.is_empty(): - fallback_create_path = path + # Creating below a directory the sandbox hides would land on its + # private tmpfs and vanish with the editor, so a hidden candidate can + # be neither evidence nor a create target. + var hidden_app_dir := McpPathTemplate.hidden_flatpak_app_dir(path) + if not hidden_app_dir.is_empty(): + hidden_paths.append(path) + if not hidden_app_dirs.has(hidden_app_dir): + hidden_app_dirs.append(hidden_app_dir) + continue + create_templates.append(template) + create_paths.append(path) _clear_config_path_warning() - return {"path": fallback_create_path, "error": ""} + var evident_create_path := _evident_create_path(create_templates, create_paths) + if not evident_create_path.is_empty(): + return {"path": evident_create_path, "error": ""} + var fallback_create_path := "" if create_paths.is_empty() else create_paths[0] + if hidden_app_dirs.is_empty(): + return {"path": fallback_create_path, "error": ""} + var hidden_error := hidden_flatpak_config_error( + display_name, fallback_create_path.get_base_dir(), hidden_app_dirs + ) + if fallback_create_path.is_empty(): + return {"path": "", "error": hidden_error} + return { + "path": fallback_create_path, + "error": "", + "create_error": hidden_error, + "hidden_paths": hidden_paths, + } + + +## The location the client has demonstrably run from, among candidates with no +## config file yet; "" when nothing says. Candidates are compared level by +## level, from the directory that would hold the file upward, and the first +## level at which one of those directories exists decides, in descriptor order. +## A client nested in its host app's settings (a VS Code extension) therefore +## follows the host app when the extension's own directory exists nowhere, +## instead of creating a second, unread copy of the host's settings tree. +## +## Only directories the templates name below their shared root are compared, +## and only as many levels as the shortest template has. Those belong to the +## client; a container such as ~/.config exists whether or not the client does. +static func _evident_create_path(templates: PackedStringArray, paths: PackedStringArray) -> String: + if paths.is_empty(): + return "" + var shared := _shared_root_length(templates) + var levels := -1 + for template in templates: + var own_directories := template.split("/").size() - 1 - shared + levels = own_directories if levels < 0 else mini(levels, own_directories) + var directories := PackedStringArray() + for path in paths: + directories.append(path.get_base_dir()) + for _level in range(maxi(levels, 0)): + for index in range(directories.size()): + if DirAccess.dir_exists_absolute(directories[index]): + return paths[index] + for index in range(directories.size()): + directories[index] = directories[index].get_base_dir() + return "" + + +## How many leading path elements every template shares, counting the root +## element (`$XDG_CONFIG_HOME`, `~`, a drive) and never the file name. At least +## one: the root is never the client's own directory. +static func _shared_root_length(templates: PackedStringArray) -> int: + if templates.size() < 2: + return 1 + var first := templates[0].split("/") + var shared := first.size() - 1 + for index in range(1, templates.size()): + var other := templates[index].split("/") + var limit := mini(shared, other.size() - 1) + var same := 0 + while same < limit and first[same] == other[same]: + same += 1 + shared = same + return maxi(shared, 1) + + +## Why Configure will not create a client's settings file from inside a +## Flatpak editor: nothing shows the client runs from the ordinary location, +## and the location a Flatpak build of it uses cannot be inspected. Names the +## one grant that lets this editor see it. +static func hidden_flatpak_config_error( + client_name: String, settings_dir: String, hidden_app_dirs: PackedStringArray +) -> String: + var app_id := McpPathTemplate.flatpak_app_id() + var grants := PackedStringArray() + for app_dir in hidden_app_dirs: + grants.append("--filesystem=%s" % app_dir) + var missing := ( + "%s does not exist" % settings_dir + if not settings_dir.is_empty() + else "it has no settings location outside Flatpak" + ) + return ( + "Could not find where %s keeps its settings: %s, and a Flatpak build " + + "keeps them under %s, which Godot's own Flatpak sandbox cannot see. " + + "If you use the Flatpak build, run `flatpak override --user %s %s`, " + + "restart Godot, and configure again. If it is installed outside " + + "Flatpak, start it once so it creates its settings, then configure " + + "again." + ) % [ + client_name, missing, ", ".join(hidden_app_dirs), + " ".join(grants), app_id if not app_id.is_empty() else "", + ] ## Find a readable seed for a newly-created authoritative wildcard target. diff --git a/addons/godot_ai/clients/_cli_finder.gd b/addons/godot_ai/clients/_cli_finder.gd index b22f508..cc1668c 100644 --- a/addons/godot_ai/clients/_cli_finder.gd +++ b/addons/godot_ai/clients/_cli_finder.gd @@ -25,6 +25,8 @@ static var _cache: Dictionary = {} # exe_name -> resolved path (or "") static var _searched: Dictionary = {} const _LOOKUP_TIMEOUT_MS := 3000 +const _TRACE_PATH_LIMIT := 16 +const _TRACE_PATH_MAX_CHARS := 512 ## Find any of the supplied exe names; returns the first hit. @@ -56,7 +58,15 @@ static func _find_one(exe_name: String, trace: Callable = Callable()) -> String: var cached: String = _cache.get(exe_name, "") _mutex.unlock() if already_searched: - _trace_lookup(trace, "cache", "resolved" if not cached.is_empty() else "cached_miss", started, {}, true) + _trace_lookup( + trace, + "cache", + "resolved" if not cached.is_empty() else "cached_miss", + started, + {}, + true, + cached, + ) return cached # `_resolve()` does FileAccess + bounded subprocess lookup (forks # `bash -lc` / `which`), which can take 100ms-1s. Holding the mutex across that @@ -76,12 +86,14 @@ static func _resolve(exe_name: String, trace: Callable = Callable()) -> String: var is_windows := OS.get_name() == "Windows" # 1. Well-known locations + var well_known_paths := PackedStringArray() for dir in _well_known_dirs(): var full := dir.path_join(exe_name) + well_known_paths.append(full) if FileAccess.file_exists(full): - _trace_lookup(trace, "well_known", "resolved", started) + _trace_lookup(trace, "well_known", "resolved", started, {}, false, full, well_known_paths) return full - _trace_lookup(trace, "well_known", "no_match", started) + _trace_lookup(trace, "well_known", "no_match", started, {}, false, "", well_known_paths) # 2. Login shell lookup (Unix only) if not is_windows: @@ -97,22 +109,50 @@ static func _resolve(exe_name: String, trace: Callable = Callable()) -> String: if int(login_result.get("exit_code", -1)) == 0: var login_found: String = str(login_result.get("stdout", "")).strip_edges() if not login_found.is_empty() and FileAccess.file_exists(login_found): - _trace_lookup(trace, "login_shell", "resolved", started, login_result) + _trace_lookup( + trace, + "login_shell", + "resolved", + started, + login_result, + false, + login_found, + PackedStringArray([login_found]), + ) return login_found _trace_lookup(trace, "login_shell", _lookup_failure(login_result, false), started, login_result) # 3. which / where with inherited PATH + return _resolve_inherited_path(exe_name, is_windows, trace) + + +## Resolve from the inherited PATH through a single lookup command. The +## optional runner keeps the Windows/where failure modes deterministic in unit +## tests without changing the production invocation. +static func _resolve_inherited_path( + exe_name: String, + is_windows: bool, + trace: Callable = Callable(), + lookup_runner: Callable = Callable(), +) -> String: var lookup := "where" if is_windows else "which" - started = Time.get_ticks_msec() - var result := McpCliExec.run(lookup, [exe_name], _LOOKUP_TIMEOUT_MS, false) + var started := Time.get_ticks_msec() + var result: Dictionary = {} + if lookup_runner.is_valid(): + var injected: Variant = lookup_runner.call(lookup, exe_name) + if injected is Dictionary: + result = injected + else: + result = McpCliExec.run(lookup, [exe_name], _LOOKUP_TIMEOUT_MS, false) + var lines := PackedStringArray(str(result.get("stdout", "")).split("\n")) if int(result.get("exit_code", -1)) == 0: - var output := str(result.get("stdout", "")) - var lines := PackedStringArray(output.split("\n")) var found := _pick_best_path(lines) if is_windows else lines[0].strip_edges() if not found.is_empty(): - _trace_lookup(trace, "inherited_path", "resolved", started, result) + _trace_lookup(trace, "inherited_path", "resolved", started, result, false, found, lines) return found - _trace_lookup(trace, "inherited_path", _lookup_failure(result, true), started, result) + _trace_lookup( + trace, "inherited_path", _lookup_failure(result, true), started, result, false, "", lines + ) return "" @@ -128,7 +168,8 @@ static func _lookup_failure(result: Dictionary, absence_exit: bool) -> String: static func _trace_lookup( trace: Callable, tier: String, status: String, started: int, - result: Dictionary = {}, cache_hit := false, + result: Dictionary = {}, cache_hit := false, resolved_path := "", + candidate_paths: PackedStringArray = PackedStringArray(), ) -> void: if not trace.is_valid(): return @@ -141,9 +182,44 @@ static func _trace_lookup( "spawn_failed": bool(result.get("spawn_failed", false)), "cancelled": bool(result.get("cancelled", false)), "termination_failed": bool(result.get("termination_failed", false)), + "resolved_path": _bounded_trace_path(resolved_path), + "candidate_paths": _bounded_trace_paths(candidate_paths), }) +## Keep lookup diagnostics useful without retaining arbitrary command output or +## environment values. Only absolute executable paths cross this boundary. +static func _bounded_trace_paths(paths: PackedStringArray) -> Array[String]: + var bounded: Array[String] = [] + for raw in paths: + var path := _bounded_trace_path(raw) + if path.is_empty() or bounded.has(path): + continue + bounded.append(path) + if bounded.size() >= _TRACE_PATH_LIMIT: + break + return bounded + + +static func _bounded_trace_path(raw: String) -> String: + var path := raw.strip_edges() + if not _is_traceable_path(path): + return "" + return path.left(_TRACE_PATH_MAX_CHARS) + + +static func _is_traceable_path(path: String) -> bool: + if path.is_empty() or path.contains("\n") or path.contains("\r"): + return false + if path.begins_with("/") or path.begins_with("\\\\"): + return true + return ( + path.length() >= 3 + and path.substr(1, 1) == ":" + and (path.substr(2, 1) == "\\" or path.substr(2, 1) == "/") + ) + + ## Executable extensions Windows' CreateProcessW can launch from a path ## (after the cmd.exe wrap in `_cli_exec.gd`). Order is preference: `.exe` ## is a native PE binary; `.cmd` / `.bat` go through the shell; `.com` is diff --git a/addons/godot_ai/clients/_cli_strategy.gd b/addons/godot_ai/clients/_cli_strategy.gd index 2d2d013..000c59c 100644 --- a/addons/godot_ai/clients/_cli_strategy.gd +++ b/addons/godot_ai/clients/_cli_strategy.gd @@ -30,6 +30,9 @@ static func configure( server_url: String, launch: Dictionary = {}, ) -> Dictionary: + var write_error := configure_write_error(client) + if not write_error.is_empty(): + return {"status": "error", "message": write_error} ## Fail closed before any subprocess runs: a command-shape client without a ## verified attach launcher must not register anything (see ## docs/client-configuration.md — an ERROR beats an entry known to be broken). @@ -184,6 +187,9 @@ static func _status_details( static func remove(client: McpClient, server_name: String) -> Dictionary: + var write_error := remove_write_error(client) + if not write_error.is_empty(): + return {"status": "error", "message": write_error} var cli := _resolve_cli(client) if cli.is_empty(): return {"status": "error", "message": "%s not found" % client.display_name} @@ -440,6 +446,33 @@ static func uses_scope_token(client: McpClient) -> bool: return client.cli_register_template.has(SCOPE_TOKEN) +## Why Configure may not run this client's CLI. The CLI runs inside this +## editor's Flatpak sandbox, so what it puts in the file the client keeps its +## servers in (the JSON-fallback file) lands where the client outside never +## reads it. That file takes the `user` scope's entry and the `local` scope's +## per-project block, and the pre-cleanup sweep (#872) removes from it at +## every scope, so the selected scope makes no difference here: at `project` +## the sweep would miss a stale entry in the host's file and Configure would +## still report success. A descriptor that names no such file has nothing to +## check. +static func configure_write_error(client: McpClient) -> String: + if not client.has_json_fallback(): + return "" + return client.config_write_error() + + +## Why Remove may not. It removes from the selected scope only: `user` and +## `local` keep their entries in the JSON-fallback file, and `project` in the +## CLI's working directory, which this layer does not resolve and leaves to +## the CLI. +static func remove_write_error(client: McpClient) -> String: + if not client.has_json_fallback(): + return "" + if uses_scope_token(client) and McpSettings.client_scope() == "project": + return "" + return client.config_write_error() + + ## Public view of the pre-cleanup sweep, for the manual-command text: what the ## dock tells the user to run has to match what Configure actually runs (#877). static func cleanup_scopes(client: McpClient) -> Array[String]: diff --git a/addons/godot_ai/clients/_dsh_strategy.gd b/addons/godot_ai/clients/_dsh_strategy.gd index 4d86c72..5df8f6f 100644 --- a/addons/godot_ai/clients/_dsh_strategy.gd +++ b/addons/godot_ai/clients/_dsh_strategy.gd @@ -64,10 +64,16 @@ static func configure( var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty(): return {"status": "error", "message": "Could not resolve config path for %s on this OS" % client.display_name} + ## Set only while no candidate file exists, so this never blocks an update. + var create_error := str(resolution.get("create_error", "")) + if not create_error.is_empty(): + return {"status": "error", "message": create_error} ## Fail closed before touching the file — same contract as JSON/TOML/YAML. var launch_error := command_launch_error(client, launch) if not launch_error.is_empty(): @@ -191,6 +197,8 @@ static func remove(client: McpClient, server_name: String) -> Dictionary: var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty() or not FileAccess.file_exists(path): diff --git a/addons/godot_ai/clients/_json_strategy.gd b/addons/godot_ai/clients/_json_strategy.gd index 9930416..696e3a6 100644 --- a/addons/godot_ai/clients/_json_strategy.gd +++ b/addons/godot_ai/clients/_json_strategy.gd @@ -19,10 +19,16 @@ static func configure( var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty(): return {"status": "error", "message": "Could not resolve config path for %s on this OS" % client.display_name} + ## Set only while no candidate file exists, so this never blocks an update. + var create_error := str(resolution.get("create_error", "")) + if not create_error.is_empty(): + return {"status": "error", "message": create_error} var seed_path := str(resolution.get("seed_path", "")) var read_path := seed_path if not FileAccess.file_exists(path) and not seed_path.is_empty() else path @@ -59,6 +65,9 @@ static func _configure_merged( launch: Dictionary, project_roots: PackedStringArray, ) -> Dictionary: + var write_error := _merge_write_error(client) + if not write_error.is_empty(): + return {"status": "error", "message": write_error} var launch_error := command_launch_error(client, launch) if not launch_error.is_empty(): return {"status": "error", "message": launch_error} @@ -259,6 +268,8 @@ static func remove( var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty() or not FileAccess.file_exists(path): @@ -284,6 +295,9 @@ static func remove( static func _remove_merged( client: McpClient, server_name: String, project_roots: PackedStringArray ) -> Dictionary: + var write_error := _merge_write_error(client) + if not write_error.is_empty(): + return {"status": "error", "message": write_error} ## Scope ambiguity (omp named profiles) blocks Remove too: clearing the ## default files while a profile reads its own would report a success the ## running client never observes (#1085). @@ -750,6 +764,21 @@ static func _merge_paths(client: McpClient) -> PackedStringArray: return result +## The Flatpak write check `configure` and `remove` make on their one path, +## made here on every tier. Any tier this sandbox does not share with the host +## refuses the whole Configure or Remove: an unseen tier may hold the +## definition the client actually uses. Status reads past it, as it does for +## an ordinary client. +static func _merge_write_error(client: McpClient) -> String: + for path in _merge_paths(client): + if not path.is_absolute_path(): + continue + var write_error := McpClient.unshared_flatpak_config_error(client.display_name, path) + if not write_error.is_empty(): + return write_error + return "" + + ## Pi's project tiers are relative to Pi's process cwd, not Godot's. Inspect ## plausible roots only to detect overrides; callers fail closed rather than ## mutating both guesses. diff --git a/addons/godot_ai/clients/_path_template.gd b/addons/godot_ai/clients/_path_template.gd index 8fc9753..fc870ba 100644 --- a/addons/godot_ai/clients/_path_template.gd +++ b/addons/godot_ai/clients/_path_template.gd @@ -23,6 +23,19 @@ extends RefCounted static var _env_snapshot := {} static var _env_snapshot_mutex := Mutex.new() +const LinuxProc := preload("res://addons/godot_ai/utils/linux_proc.gd") +const _MOUNTINFO_PATH := "/proc/self/mountinfo" + +## `/.flatpak-info` as Flatpak wrote it before this process started, or "" when +## this is not a Flatpak sandbox. The sandbox it describes cannot change while +## the process runs, so one read serves every thread. Guarded by +## `_env_snapshot_mutex`, like the snapshot it is consulted alongside. +static var _flatpak_info := "" +static var _flatpak_info_read := false +## What that sandbox mounted, from `/proc/self/mountinfo`: read with the file +## above, and only inside a sandbox. Empty when it could not be read. +static var _flatpak_mounts: Array[Dictionary] = [] + ## Every var this layer and its sibling consumers (`_base.gd` ## `config_file_override_details`, `config_home_override_details`, `_cli_finder.gd` lookups, ## `client_configurator.gd` mode/trace reads) can touch off-main. @@ -32,6 +45,10 @@ const _BASE_ENV_VARS: Array[String] = [ "HOME", "USERPROFILE", "XDG_CONFIG_HOME", + ## What `$XDG_CONFIG_HOME` reads in a Flatpak editor — see `_env_source`. + "HOST_XDG_CONFIG_HOME", + ## Read by `flatpak_hides_credentials` on the workers that run Configure. + "GODOT_AI_CAPABILITY_DIR", "APPDATA", "LOCALAPPDATA", "SHELL", @@ -76,6 +93,120 @@ static func warm_env_snapshot(extra_vars: PackedStringArray = PackedStringArray( for var_name in extra_vars: if not String(var_name).is_empty(): env_lookup(String(var_name)) + _flatpak_info_text() + + +## Cached `/.flatpak-info`, read on first use from whichever thread asks. Only +## Linux has Flatpak; elsewhere the leading slash would name a drive root. +static func _flatpak_info_text() -> String: + _env_snapshot_mutex.lock() + _read_flatpak_sandbox() + var info := _flatpak_info + _env_snapshot_mutex.unlock() + return info + + +## The sandbox's mounts, cached with the description above. The list is built +## once and never changed, so callers on any thread may read it. +static func _flatpak_mount_list() -> Array[Dictionary]: + _env_snapshot_mutex.lock() + _read_flatpak_sandbox() + var mounts := _flatpak_mounts + _env_snapshot_mutex.unlock() + return mounts + + +## Caller holds `_env_snapshot_mutex`. +static func _read_flatpak_sandbox() -> void: + if _flatpak_info_read: + return + _flatpak_info = McpTransportCapability._flatpak_info() if _os_key() == "linux" else "" + var mountinfo := "" + if not _flatpak_info.is_empty(): + mountinfo = str(LinuxProc.read_text(_MOUNTINFO_PATH).text) + _flatpak_mounts = McpTransportCapability.parse_mountinfo(mountinfo) + _flatpak_info_read = true + + +## Stand in for the sandbox description so tests can cover Flatpak resolution +## on any host, with `mountinfo` as its `/proc/self/mountinfo`: by default no +## mount evidence at all, never the real file's. Pass null to forget both; the +## next read then asks the real files. +static func _set_flatpak_info_for_test(info: Variant, mountinfo: String = "") -> void: + _env_snapshot_mutex.lock() + _flatpak_info = "" if info == null else str(info) + _flatpak_mounts = McpTransportCapability.parse_mountinfo(mountinfo) + _flatpak_info_read = info != null + _env_snapshot_mutex.unlock() + + +## This editor's Flatpak application ID, or "" outside Flatpak. +static func flatpak_app_id() -> String: + return McpTransportCapability.flatpak_application_id(_flatpak_info_text()) + + +## The host's config directory when this editor is a Flatpak that shares it +## (through the home directory, or a grant for the whole of `xdg-config`); "" +## anywhere else, and when it cannot be resolved. For callers whose own +## default is `OS.get_config_dir()`, which in that sandbox is the per-app +## directory that nothing outside it reads. +static func flatpak_host_config_home() -> String: + if not McpTransportCapability.flatpak_shares_config_home(_flatpak_info_text()): + return "" + var config_home := expand("$XDG_CONFIG_HOME") + return config_home if config_home.is_absolute_path() else "" + + +## Whether this editor's Flatpak sandbox keeps Godot AI's credentials where a +## client outside it does not look. The bridge a client starts reads the +## host's `/godot-ai/capabilities`, so an entry Configure writes +## from such a sandbox cannot connect, however right its file is. +## +## The server publishes in the host's config directory when the sandbox +## shares it (`McpTransportCapability.linux_config_home_variable`), and +## otherwise in the per-app one, which is the host's only where an +## `xdg-config/` grant mounted the host's directory inside it. Either way the +## host's directory has to be shared read-write, which `flatpak_write_block` +## decides. An editor given `GODOT_AI_CAPABILITY_DIR` publishes somewhere this +## cannot judge, and its client needs the same variable: never hidden here. +static func flatpak_hides_credentials() -> bool: + var info := _flatpak_info_text() + if info.is_empty(): + return false + if not env_lookup(McpTransportCapability.CAPABILITY_DIR_ENV).strip_edges().is_empty(): + return false + var config_home := expand("$XDG_CONFIG_HOME") + if not config_home.is_absolute_path(): + return false + var host_directory := config_home.path_join("godot-ai/capabilities") + if not flatpak_write_block(host_directory.path_join("http.json")).is_empty(): + return true + if McpTransportCapability.flatpak_shares_config_home(info): + return false + var grant := McpTransportCapability.flatpak_grant_over( + info, host_directory, _home(), config_home + ) + return not str(grant.get("location", "")).begins_with("xdg-config/") + + +## The `~/.var/app/` directory of another Flatpak app that `path` lies in, +## when this editor's own Flatpak sandbox does not show it; "" otherwise. +## Flatpak mounts only the running app's directory under `~/.var/app`, and a +## `host` or `home` grant does not widen that. From in here another app's +## settings are therefore unknown rather than absent until the user grants +## that directory by name, and anything created below it would land on the +## sandbox's private tmpfs. +static func hidden_flatpak_app_dir(path: String) -> String: + if _flatpak_info_text().is_empty(): + return "" + var home := _home() + if home.is_empty(): + return "" + var apps := home.path_join(".var/app") + "/" + if not path.begins_with(apps): + return "" + var app_dir := apps + path.trim_prefix(apps).get_slice("/", 0) + return "" if DirAccess.dir_exists_absolute(app_dir) else app_dir ## Pick the right entry from a {"darwin": ..., "windows": ..., "linux": ...} map. @@ -183,7 +314,7 @@ static func expand(template: String) -> String: var token := "$%s" % var_name if out.find(token) < 0: continue - var value := env_lookup(var_name) + var value := env_lookup(_env_source(var_name)) if value.is_empty(): value = _home_fallback(var_name) if value.is_empty(): @@ -192,6 +323,81 @@ static func expand(template: String) -> String: return out +## The environment variable a token's value is read from. Flatpak points +## `XDG_CONFIG_HOME` at the editor's own `~/.var/app//config`, where no +## client looks: a path expanded from it is written, read back, and reported +## as configured without the client ever seeing it. Inside a sandbox the token +## therefore reads the host's variable, so a template always names the file +## the client reads. Whether this sandbox can reach that file is a separate +## question, answered by `flatpak_write_block`. Every grant that shares part +## of the host's config directory mounts it at the host's path, an +## `xdg-config/` grant included, so no path has to go through the per-app +## directory, where a leftover from an earlier run would pass for the client's +## own file. +static func _env_source(var_name: String) -> String: + if var_name == "XDG_CONFIG_HOME" and not _flatpak_info_text().is_empty(): + return "HOST_XDG_CONFIG_HOME" + return var_name + + +## Why a file this editor writes at `path` would not be the one a client +## outside its Flatpak sandbox reads there. {} when it would be, and always +## outside Flatpak. Otherwise one of: +## - {"needs": grant}: no read-write grant covers the file's directory, and +## this is the `--filesystem=` value that would. That is the read-only grant +## that does cover it when there is one, because a wider grant does not +## lift it. +## - {"unmounted": directory}: a read-write grant names this directory, but +## Flatpak mounted nothing there, which is what it does when the directory +## does not exist as the app starts. +## - {"hidden": directory}: inside a granted directory, this one is an empty +## tmpfs, which is how Flatpak carries out a denial (`--nofilesystem`). +## - {"read_only": directory}: granted read-write, mounted read-only. +## +## The directory is what has to be shared, not the file: `McpAtomicWrite` +## stages a temporary file beside the target and renames it into place, which +## cannot replace a file Flatpak mounted on its own. It also follows a +## symlinked config file to its target, so that is the file tested. +static func flatpak_write_block(path: String) -> Dictionary: + var info := _flatpak_info_text() + if info.is_empty(): + return {} + var directory := McpAtomicWrite._resolve_symlink_target(path).get_base_dir() + var home := _home() + var inside_home := ( + not home.is_empty() and McpTransportCapability.path_is_within(home, directory) + ) + var grant := McpTransportCapability.flatpak_grant_over( + info, directory, home, expand("$XDG_CONFIG_HOME") + ) + if not grant.get("writable", false): + if not grant.is_empty(): + return {"needs": str(grant["location"])} + ## Nothing under ~/.var/app comes with the home, so there the app's + ## directory has to be granted by name. + var apps := home.path_join(".var/app") + "/" + if inside_home and directory.begins_with(apps): + return {"needs": apps + directory.trim_prefix(apps).get_slice("/", 0)} + return {"needs": "home" if inside_home else directory} + var root := str(grant["root"]) + var real_root := McpTransportCapability.real_path(root) + var mount := McpTransportCapability.flatpak_blocking_mount( + _flatpak_mount_list(), + McpTransportCapability.real_path(directory), + McpTransportCapability.real_path(home) if inside_home else real_root, + ) + if mount.is_empty(): + return {} + var mount_point := str(mount["mount_point"]) + if mount["read_only"]: + return {"read_only": mount_point} + ## A tmpfs above the granted directory is not there because of the grant: + ## the grant's own mount is what is missing. + if not McpTransportCapability.path_is_within(real_root, mount_point): + return {"unmounted": root} + return {"hidden": mount_point} + + ## Home-derived default for a var the environment does not define. Returns "" ## when home itself is unresolvable so `expand` leaves the token alone: rooting ## these at an empty home yields a RELATIVE path, not an absent one — diff --git a/addons/godot_ai/clients/_toml_strategy.gd b/addons/godot_ai/clients/_toml_strategy.gd index 063f387..93272f0 100644 --- a/addons/godot_ai/clients/_toml_strategy.gd +++ b/addons/godot_ai/clients/_toml_strategy.gd @@ -19,10 +19,16 @@ static func configure( var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty(): return {"status": "error", "message": "Could not resolve config path for %s" % client.display_name} + ## Set only while no candidate file exists, so this never blocks an update. + var create_error := str(resolution.get("create_error", "")) + if not create_error.is_empty(): + return {"status": "error", "message": create_error} var seed_path := str(resolution.get("seed_path", "")) var read_path := seed_path if not FileAccess.file_exists(path) and not seed_path.is_empty() else path @@ -198,6 +204,8 @@ static func remove(client: McpClient, _server_name: String) -> Dictionary: var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty() or not FileAccess.file_exists(path): diff --git a/addons/godot_ai/clients/_yaml_strategy.gd b/addons/godot_ai/clients/_yaml_strategy.gd index becf4ac..617cf16 100644 --- a/addons/godot_ai/clients/_yaml_strategy.gd +++ b/addons/godot_ai/clients/_yaml_strategy.gd @@ -24,10 +24,16 @@ static func configure( var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty(): return {"status": "error", "message": "Could not resolve config path for %s on this OS" % client.display_name} + ## Set only while no candidate file exists, so this never blocks an update. + var create_error := str(resolution.get("create_error", "")) + if not create_error.is_empty(): + return {"status": "error", "message": create_error} ## Fail closed before touching the file — same contract as JSON/TOML. var launch_error := command_launch_error(client, launch) if not launch_error.is_empty(): @@ -105,6 +111,8 @@ static func remove(client: McpClient, server_name: String) -> Dictionary: var resolution := client.resolved_config_path_details() var path := str(resolution.get("path", "")) var path_error := str(resolution.get("error", "")) + if path_error.is_empty() and not path.is_empty(): + path_error = McpClient.unshared_flatpak_config_error(client.display_name, path) if not path_error.is_empty(): return {"status": "error", "message": path_error} if path.is_empty() or not FileAccess.file_exists(path): diff --git a/addons/godot_ai/clients/cline.gd b/addons/godot_ai/clients/cline.gd index 9e827b6..b61a7cc 100644 --- a/addons/godot_ai/clients/cline.gd +++ b/addons/godot_ai/clients/cline.gd @@ -14,6 +14,14 @@ func _init() -> void: "windows": "$APPDATA/Code/User/globalStorage/saoudrizwan.claude-dev/settings/cline_mcp_settings.json", "linux": "$XDG_CONFIG_HOME/Code/User/globalStorage/saoudrizwan.claude-dev/settings/cline_mcp_settings.json", } + ## globalStorage follows VS Code's user data, which the Flathub build keeps + ## in its own app directory (see vscode.gd). + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/com.visualstudio.code/config/Code/User/globalStorage/saoudrizwan.claude-dev/settings/cline_mcp_settings.json", + ], + } server_key_path = PackedStringArray(["mcpServers"]) ## Cline (like Roo) defaults a typeless entry to SSE transport, which ## returns HTTP 400 against our streamable-http endpoint on `/mcp`. Pin diff --git a/addons/godot_ai/clients/kilo_code.gd b/addons/godot_ai/clients/kilo_code.gd index b904837..7c477e6 100644 --- a/addons/godot_ai/clients/kilo_code.gd +++ b/addons/godot_ai/clients/kilo_code.gd @@ -11,6 +11,14 @@ func _init() -> void: "windows": "$APPDATA/Code/User/globalStorage/kilocode.kilo-code/settings/mcp_settings.json", "linux": "$XDG_CONFIG_HOME/Code/User/globalStorage/kilocode.kilo-code/settings/mcp_settings.json", } + ## globalStorage follows VS Code's user data, which the Flathub build keeps + ## in its own app directory (see vscode.gd). + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/com.visualstudio.code/config/Code/User/globalStorage/kilocode.kilo-code/settings/mcp_settings.json", + ], + } server_key_path = PackedStringArray(["mcpServers"]) ## Kilo Code (like Roo) defaults a typeless entry to SSE transport, which ## returns HTTP 400 against our streamable-http endpoint on `/mcp`. Pin diff --git a/addons/godot_ai/clients/roo_code.gd b/addons/godot_ai/clients/roo_code.gd index 5a1e697..399931d 100644 --- a/addons/godot_ai/clients/roo_code.gd +++ b/addons/godot_ai/clients/roo_code.gd @@ -11,6 +11,14 @@ func _init() -> void: "windows": "$APPDATA/Code/User/globalStorage/rooveterinaryinc.roo-cline/settings/mcp_settings.json", "linux": "$XDG_CONFIG_HOME/Code/User/globalStorage/rooveterinaryinc.roo-cline/settings/mcp_settings.json", } + ## globalStorage follows VS Code's user data, which the Flathub build keeps + ## in its own app directory (see vscode.gd). + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/com.visualstudio.code/config/Code/User/globalStorage/rooveterinaryinc.roo-cline/settings/mcp_settings.json", + ], + } server_key_path = PackedStringArray(["mcpServers"]) ## Roo defaults an entry with no "type" to SSE transport — which returns ## HTTP 400 against our streamable-http endpoint on `/mcp`. Pin the type diff --git a/addons/godot_ai/clients/vscode.gd b/addons/godot_ai/clients/vscode.gd index 9b97c53..5f9c59e 100644 --- a/addons/godot_ai/clients/vscode.gd +++ b/addons/godot_ai/clients/vscode.gd @@ -14,6 +14,15 @@ func _init() -> void: "windows": "$APPDATA/Code/User/mcp.json", "linux": "$XDG_CONFIG_HOME/Code/User/mcp.json", } + ## The Flathub build keeps its user data in the app's own directory and + ## never reads ~/.config/Code. Verified against com.visualstudio.code + ## 1.139.1: `code --add-mcp` writes the second path, under `servers`. + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/com.visualstudio.code/config/Code/User/mcp.json", + ], + } server_key_path = PackedStringArray(["servers"]) entry_extra_fields = {"type": "http"} ## Attach migration (#838). VS Code stdio entries are flat command/args/env diff --git a/addons/godot_ai/clients/zed.gd b/addons/godot_ai/clients/zed.gd index e189064..dbcf4ac 100644 --- a/addons/godot_ai/clients/zed.gd +++ b/addons/godot_ai/clients/zed.gd @@ -21,6 +21,16 @@ func _init() -> void: "linux": "$XDG_CONFIG_HOME/zed/settings.json", "windows": "$APPDATA/Zed/settings.json", } + ## The Flathub build restarts itself on the host with + ## FLATPAK_XDG_CONFIG_HOME set to its own app directory, and reads + ## `$FLATPAK_XDG_CONFIG_HOME/zed` instead of ~/.config/zed + ## (crates/paths/src/paths.rs, crates/cli/src/main.rs). + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/dev.zed.Zed/config/zed/settings.json", + ], + } server_key_path = PackedStringArray(["context_servers"]) ## Attach migration (#838). Current Zed's context_servers entries are an ## untagged serde enum discriminated by shape: `command` (string) + args/env diff --git a/addons/godot_ai/clients/zoo_code.gd b/addons/godot_ai/clients/zoo_code.gd index 85529bf..657233c 100644 --- a/addons/godot_ai/clients/zoo_code.gd +++ b/addons/godot_ai/clients/zoo_code.gd @@ -11,6 +11,14 @@ func _init() -> void: "windows": "$APPDATA/Code/User/globalStorage/zoocodeorganization.zoo-code/settings/mcp_settings.json", "linux": "$XDG_CONFIG_HOME/Code/User/globalStorage/zoocodeorganization.zoo-code/settings/mcp_settings.json", } + ## globalStorage follows VS Code's user data, which the Flathub build keeps + ## in its own app directory (see vscode.gd). + config_path_candidates = { + "linux": [ + path_template["linux"], + "~/.var/app/com.visualstudio.code/config/Code/User/globalStorage/zoocodeorganization.zoo-code/settings/mcp_settings.json", + ], + } server_key_path = PackedStringArray(["mcpServers"]) ## Local validation against the installed extension shows Zoo stores MCP ## entries in `settings/mcp_settings.json` under `mcpServers`, matching Roo's diff --git a/addons/godot_ai/handlers/filesystem_mutation.gd b/addons/godot_ai/handlers/filesystem_mutation.gd index aac8fb3..45163f6 100644 --- a/addons/godot_ai/handlers/filesystem_mutation.gd +++ b/addons/godot_ai/handlers/filesystem_mutation.gd @@ -10,17 +10,30 @@ const MAX_ENTRIES := 10000 const MAX_FILE_BYTES := 256 * 1024 const MAX_TOTAL_BYTES := 64 * 1024 * 1024 const MAX_TARGETS := 256 -const BUDGET_USEC := 2000 +## Container magic Godot writes for a compressed binary resource. +const COMPRESSED_MAGIC := "RSCC" +## Work allowed between frame yields: a share of how long the editor last took +## to come back, within these bounds. A fixed slice starves discovery once the +## editor is unfocused, because its low-processor mode then sleeps ~100 ms per +## frame (`interface/editor/timers/unfocused_low_processor_mode_sleep_usec`) +## and the frame count, not the work, reaches the deadline (#1120). The floor is +## what a focused editor gets; the ceiling bounds one slice on a stalled editor. +const BUDGET_MIN_USEC := 2000 +const BUDGET_MAX_USEC := 50000 +const BUDGET_GAP_DIVISOR := 3 const DEADLINE_MSEC := 25000 static var _busy := false var _alive: Callable var _deadline := 0 var _yield_at := 0 +var _budget := BUDGET_MIN_USEC var _bytes := 0 var _directories: Dictionary = {} var _fingerprints: Dictionary = {} +var _large_owners: Dictionary = {} var _fault := "" +var _fault_code := "" var _io: Callable ## Dispatch stays synchronous; the retained job owns work across frame yields. @@ -59,6 +72,13 @@ func run(params: Dictionary, operation: String, active: Callable = Callable()) - _alive = active _deadline = Time.get_ticks_msec() + DEADLINE_MSEC _yield_at = Time.get_ticks_usec() + _budget = BUDGET_MIN_USEC + _bytes = 0 + _directories.clear() + _fingerprints.clear() + _large_owners.clear() + _fault = "" + _fault_code = "" var result := await _run(params, operation) _busy = false return result @@ -100,7 +120,7 @@ func _run(params: Dictionary, operation: String) -> Dictionary: return _unchanged(_fault) var files: Array[String] = [] if not await _collect_tree("res://", files): - return _unchanged(_fault) + return _fault_result() if directory and not _directories.has(source): return _unchanged("Source directory was excluded or has noncanonical casing; nothing changed") var targets := {} @@ -112,27 +132,46 @@ func _run(params: Dictionary, operation: String) -> Dictionary: if targets.size() > MAX_TARGETS: return _unchanged("Mutation exceeds the %d-resource limit; use smaller groups" % MAX_TARGETS) for path in files: - if path == source or (directory and path.begins_with(source + "/")) or path in [source + ".uid", source + ".import"]: + if not _mutation_member(path, source, directory): + continue + if path.get_extension().to_lower() in OWNER_EXTENSIONS: # Owner-extension targets are read and fingerprinted by the owner # pass below; reading them here too would charge the byte budget twice. - if path.get_extension().to_lower() in OWNER_EXTENSIONS: - continue - if await _read_bounded(path) == null: - return _unchanged(_fault) + continue + if await _read_bounded(path) == null: + return _fault_result() var owners := [] for path in files: - if path.get_extension().to_lower() not in OWNER_EXTENSIONS: + var extension := path.get_extension().to_lower() + if extension not in OWNER_EXTENSIONS: continue - var content: Variant = await _read_bounded(path) - if content == null: - return _unchanged(_fault) - # One call per owner: _references lowercases and regex-scans the whole - # file, so a per-target loop repeated that up to MAX_TARGETS times. - var hits := await _references(path, content, targets) + var hits: Array[Dictionary] + var length := _file_length(path) + if length < 0: + return _fault_result() + if length > MAX_FILE_BYTES: + # Dependency enumeration can replace a literal scan for packed binary + # owners, but not for scripts or text resources: those can refer to a + # target in ordinary strings the graph does not report. Fail closed. + if extension not in ["res", "scn"]: + _set_fault("Cannot safely read owner %s (exceeds %d bytes)" % [path, MAX_FILE_BYTES], Errors.FILESYSTEM_DISCOVERY_FAILED) + return _fault_result() + hits = await _large_owner_references(path, targets) + else: + var content: Variant = await _read_bounded(path) + if content == null: + return _fault_result() + if extension in ["res", "scn"]: + hits = await _binary_references(path, content, targets) + else: + # One call per owner: _references lowercases and regex-scans the + # whole file, so a per-target loop repeated that up to MAX_TARGETS + # times. + hits = await _references(path, content, targets) if not _fault.is_empty(): - return _unchanged(_fault) + return _fault_result() if not await _yield_if_needed(): - return _unchanged(_fault) + return _fault_result() if not hits.is_empty() and not (operation == "remove" and targets.has(path)): owners.append({"path": path, "references": hits}) var editor_guard := _editor_state_guard(targets) @@ -152,17 +191,18 @@ func _run(params: Dictionary, operation: String) -> Dictionary: group.append({"from": source + suffix, "to": destination + suffix}) for item in group: if not _confined_without_links(item.from): - return _unchanged(_fault) + return _fault_result() if operation != "remove" and not _destination_available(item.from, item.to): - return _unchanged(_fault) + return _fault_result() # Re-read owner bytes and directory entry censuses after the yielding plan. if not await _revalidate(): - return _unchanged(_fault) + return _fault_result() if not _active(): - return _unchanged("Filesystem request expired before mutation; nothing changed") + _set_fault("Filesystem request expired before mutation; nothing changed", Errors.FILESYSTEM_DISCOVERY_FAILED) + return _fault_result() for item in group: if not _confined_without_links(item.from) or (operation != "remove" and not _destination_available(item.from, item.to)): - return _unchanged(_fault) + return _fault_result() # Revalidation yields. Recheck mutable editor authority after the last # callback/yield, with no suspension between this guard and disk effects. editor_guard = _editor_state_guard(targets) @@ -179,17 +219,26 @@ func _active() -> bool: func _yield_if_needed() -> bool: if not _active(): - _fault = "Filesystem discovery expired or was cancelled; nothing changed" + _set_fault("Filesystem discovery expired or was cancelled; nothing changed", Errors.FILESYSTEM_DISCOVERY_FAILED) return false - if Time.get_ticks_usec() - _yield_at >= BUDGET_USEC: + var paused := Time.get_ticks_usec() + if paused - _yield_at >= _budget: await (Engine.get_main_loop() as SceneTree).process_frame _yield_at = Time.get_ticks_usec() + _budget = _frame_budget(_yield_at - paused) if not _active(): - _fault = "Filesystem discovery expired or was cancelled; nothing changed" + _set_fault("Filesystem discovery expired or was cancelled; nothing changed", Errors.FILESYSTEM_DISCOVERY_FAILED) return false return true +## The next slice for an editor that was away `gap_usec` between two slices. +## The frame sleep absorbs work shorter than itself, so a third of the gap is a +## quarter of a sleeping editor's frame and lengthens a busy one by a third. +static func _frame_budget(gap_usec: int) -> int: + return clampi(gap_usec / BUDGET_GAP_DIVISOR, BUDGET_MIN_USEC, BUDGET_MAX_USEC) + + func _collect_tree(root: String, files: Array[String]) -> bool: var pending: Array[String] = [root] var count := 0 @@ -197,11 +246,11 @@ func _collect_tree(root: String, files: Array[String]) -> bool: var path: String = pending.pop_back() var dir := DirAccess.open(path) if dir == null: - _fault = "Cannot inspect directory: %s" % path + _set_fault("Cannot inspect directory: %s" % path, Errors.FILESYSTEM_DISCOVERY_FAILED) return false dir.include_hidden = true if dir.list_dir_begin() != OK: - _fault = "Cannot list directory: %s" % path + _set_fault("Cannot list directory: %s" % path, Errors.FILESYSTEM_DISCOVERY_FAILED) return false var entries: Array[String] = [] var name := dir.get_next() @@ -222,7 +271,7 @@ func _collect_tree(root: String, files: Array[String]) -> bool: files.append(child) count += 1 if count > MAX_ENTRIES: - _fault = "Project discovery exceeds %d entries; nothing changed" % MAX_ENTRIES + _set_fault("Project discovery exceeds %d entries; nothing changed" % MAX_ENTRIES, Errors.FILESYSTEM_DISCOVERY_FAILED) dir.list_dir_end() return false if not await _yield_if_needed(): @@ -237,20 +286,23 @@ func _collect_tree(root: String, files: Array[String]) -> bool: func _read_bounded(path: String) -> Variant: var file := FileAccess.open(path, FileAccess.READ) - if file == null or file.get_length() > MAX_FILE_BYTES: - _fault = "Cannot safely read owner %s (unreadable or exceeds %d bytes)" % [path, MAX_FILE_BYTES] + if file == null: + _set_fault("Cannot safely read owner %s (unreadable)" % path, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + if file.get_length() > MAX_FILE_BYTES: + _set_fault("Cannot safely read owner %s (exceeds %d bytes)" % [path, MAX_FILE_BYTES], Errors.FILESYSTEM_DISCOVERY_FAILED) return null var bytes := PackedByteArray() var length := file.get_length() while file.get_position() < length: var part := file.get_buffer(mini(16384, length - file.get_position())) if part.is_empty(): - _fault = "Short read while inspecting %s" % path + _set_fault("Short read while inspecting %s" % path, Errors.FILESYSTEM_DISCOVERY_FAILED) return null bytes.append_array(part) _bytes += part.size() if _bytes > MAX_TOTAL_BYTES: - _fault = "Filesystem discovery exceeds its byte budget; nothing changed" + _set_fault("Filesystem discovery exceeds its byte budget; nothing changed", Errors.FILESYSTEM_DISCOVERY_FAILED) return null if not await _yield_if_needed(): return null @@ -262,11 +314,173 @@ func _read_bounded(path: String) -> Variant: return bytes +func _mutation_member(path: String, source: String, directory: bool) -> bool: + if directory: + return path == source or path.begins_with(source + "/") + return path == source or path in [source + ".uid", source + ".import"] + + +func _file_length(path: String) -> int: + var file := FileAccess.open(path, FileAccess.READ) + if file == null: + _set_fault("Cannot inspect owner metadata: %s" % path, Errors.FILESYSTEM_DISCOVERY_FAILED) + return -1 + var length := file.get_length() + file.close() + return length + + +func _owner_dependencies(path: String) -> PackedStringArray: + var dependencies := PackedStringArray() + for dependency in ResourceLoader.get_dependencies(path): + dependencies.append(str(dependency)) + return dependencies + + +## Lowercased `uid://` and path segments of the engine's dependency records. +func _dependency_segments(dependencies: PackedStringArray) -> Dictionary: + var segments: Dictionary = {} + for dependency in dependencies: + for segment in dependency.split("::", false): + segments[segment.to_lower()] = true + return segments + + +func _large_owner_references(path: String, targets: Dictionary) -> Array[Dictionary]: + var dependencies := _owner_dependencies(path) + var dependency_paths := _dependency_segments(dependencies) + _large_owners[path] = { + "modified": FileAccess.get_modified_time(path), + "size": _file_length(path), + "dependencies": dependencies, + } + if int(_large_owners[path]["size"]) < 0: + return [] + var hits: Array[Dictionary] = [] + for target: String in targets: + if not await _yield_if_needed(): + return hits + var uid: int = targets[target].uid + var uid_text := ResourceUID.id_to_text(uid) if uid != ResourceUID.INVALID_ID else "" + if dependency_paths.has(target.to_lower()) or (not uid_text.is_empty() and dependency_paths.has(uid_text.to_lower())): + # A dependency path can be a UID-backed ext_resource. Without reading + # the oversized owner we cannot prove the file reference is safe to + # rewrite, so fail closed for moves; force-remove may still use it. + hits.append({"path": target, "kind": "path"}) + return hits + + +func _binary_references(path: String, bytes: PackedByteArray, targets: Dictionary) -> Array[Dictionary]: + var hits: Array[Dictionary] = [] + var payload: Variant = _binary_scan_bytes(path, bytes) + if payload == null: + return hits + # The binary format stores an ext_resource UID as an integer, which no text + # scan can see, so the engine's dependency records are consulted as well. + var dependency_paths := _dependency_segments(_owner_dependencies(path)) + for target: String in targets: + # Keep cancellation and frame progress even when a path match continues. + if not await _yield_if_needed(): + return hits + var uid: int = targets[target].uid + var uid_text := ResourceUID.id_to_text(uid) if uid != ResourceUID.INVALID_ID else "" + var uses_path: bool = ( + _bytes_contains_ci(payload, target.to_utf8_buffer()) + or dependency_paths.has(target.to_lower()) + or (not uid_text.is_empty() and dependency_paths.has(uid_text.to_lower())) + ) + var uses_uid: bool = not uid_text.is_empty() and _bytes_contains_ci(payload, uid_text.to_utf8_buffer()) + if uses_path: + hits.append({"path": target, "kind": "path"}) + elif uses_uid: + if not ResourceUID.has_id(uid) or ResourceUID.get_id_path(uid) != target: + _set_fault("UID mapping is not authoritative for %s" % target) + return hits + hits.append({"path": target, "kind": "uid"}) + return hits + + +## The bytes a literal scan can inspect. The editor saves binary resources +## compressed by default (`filesystem/on_save/compress_binary_resources`), and +## a compressed container shows none of its serialized paths, so expand it +## first. Layout: magic, mode, block size and expanded size (u32 each), one +## u32 compressed size per block, then the blocks. +## +## Returns an empty array when the expanded payload exceeds the per-file limit +## (the caller then relies on dependency records alone, like any oversized +## binary owner) and null, with a fault, for a container that cannot be read. +func _binary_scan_bytes(path: String, bytes: PackedByteArray) -> Variant: + if bytes.size() < 4 or bytes.slice(0, 4).get_string_from_ascii() != COMPRESSED_MAGIC: + return bytes + var unreadable := "Cannot inspect compressed binary owner: %s" % path + if bytes.size() < 16: + _set_fault(unreadable, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + var mode := bytes.decode_u32(4) + var block_size := bytes.decode_u32(8) + var total := bytes.decode_u32(12) + if mode > FileAccess.COMPRESSION_BROTLI or block_size == 0: + _set_fault(unreadable, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + if total > MAX_FILE_BYTES: + return PackedByteArray() + var block_count := total / block_size + 1 + var offset := 16 + block_count * 4 + if offset > bytes.size(): + _set_fault(unreadable, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + var payload := PackedByteArray() + for index in block_count: + var compressed_size := bytes.decode_u32(16 + index * 4) + var expected := total % block_size if index == block_count - 1 else block_size + if offset + compressed_size > bytes.size(): + _set_fault(unreadable, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + if expected > 0: + var block := bytes.slice(offset, offset + compressed_size).decompress(expected, mode) + if block.size() != expected: + _set_fault(unreadable, Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + payload.append_array(block) + offset += compressed_size + _bytes += payload.size() + if _bytes > MAX_TOTAL_BYTES: + _set_fault("Filesystem discovery exceeds its byte budget; nothing changed", Errors.FILESYSTEM_DISCOVERY_FAILED) + return null + return payload + + +func _ascii_lower(value: int) -> int: + if value >= 65 and value <= 90: + return value + 32 + return value + + +func _bytes_contains_ci(haystack: PackedByteArray, needle: PackedByteArray) -> bool: + if needle.is_empty() or haystack.size() < needle.size(): + return false + var lower := _ascii_lower(needle[0]) + var upper := lower - 32 if lower >= 97 and lower <= 122 else lower + var candidates := [lower] + if candidates[0] != upper: + candidates.append(upper) + for candidate in candidates: + var position := haystack.find(candidate) + while position >= 0: + if position + needle.size() <= haystack.size(): + var matches := true + for index in needle.size(): + if _ascii_lower(haystack[position + index]) != _ascii_lower(needle[index]): + matches = false + break + if matches: + return true + position = haystack.find(candidate, position + 1) + return false + + func _references(path: String, bytes: PackedByteArray, targets: Dictionary) -> Array[Dictionary]: var hits: Array[Dictionary] = [] - if path.get_extension().to_lower() in ["res", "scn"]: - _fault = "Binary dependency discovery is unsupported for safe mutations: %s" % path - return hits # Compare paths conservatively across case-insensitive project volumes. # A case-only false positive on a sensitive volume is a safe refusal. var text := bytes.get_string_from_utf8().to_lower() @@ -358,9 +572,26 @@ func _revalidate() -> bool: if _fingerprints[path] != original[path]: _fault = "Owner changed during discovery: %s" % path return false + var original_large := _large_owners.duplicate(true) + for path in original_large: + if not await _large_owner_unchanged(path, original_large[path]): + _fault = "Owner changed during discovery: %s" % path + return false return true +func _large_owner_unchanged(path: String, original: Dictionary) -> bool: + if not FileAccess.file_exists(path): + return false + if FileAccess.get_modified_time(path) != int(original.get("modified", 0)): + return false + if _file_length(path) != int(original.get("size", -1)): + return false + if str(_owner_dependencies(path)) != str(original.get("dependencies", PackedStringArray())): + return false + return await _yield_if_needed() + + func _confined_without_links(path: String) -> bool: var dir := DirAccess.open("res://") if dir == null or not dir.has_method("is_link"): @@ -493,6 +724,17 @@ static func _is_sidecar(path: String) -> bool: return path.ends_with(".uid") or path.ends_with(".import") +func _set_fault(message: String, code: String = "") -> void: + _fault = message + _fault_code = code + + +func _fault_result() -> Dictionary: + if _fault_code == Errors.FILESYSTEM_DISCOVERY_FAILED: + return _failure(_fault, "unchanged", {}, _fault_code) + return _unchanged(_fault) + + static func _unchanged(message: String, details: Dictionary = {}) -> Dictionary: return _failure(message, "unchanged", details, Errors.INVALID_PARAMS) diff --git a/addons/godot_ai/handlers/node_handler.gd b/addons/godot_ai/handlers/node_handler.gd index 9a969e6..9bdaa86 100644 --- a/addons/godot_ai/handlers/node_handler.gd +++ b/addons/godot_ai/handlers/node_handler.gd @@ -160,6 +160,16 @@ func reparent_node(params: Dictionary) -> Dictionary: _undo_redo.add_do_method(new_parent, "add_child", node, true) _undo_redo.add_do_method(node, "set_owner", scene_root) for child in descendants: + ## Preserve intentional null owners and owners that live inside the + ## moved subtree. `remove_child` clears owners outside the subtree, so + ## only those need to be re-normalized to scene_root. The subtree is + ## still intact here because the recorded do-methods have not run yet. + ## + ## Re-owning an instance's internal nodes with scene_root flattened the + ## sub-scene on save and dropped its overrides (#1118). + var prior_owner: Node = child.owner + if prior_owner == null or prior_owner == node or node.is_ancestor_of(prior_owner): + continue _undo_redo.add_do_method(child, "set_owner", scene_root) _undo_redo.add_do_reference(node) _undo_redo.add_undo_method(new_parent, "remove_child", node) @@ -203,10 +213,14 @@ func set_property(params: Dictionary) -> Dictionary: var found := false var prop_type: int = TYPE_NIL + var prop_hint: int = PROPERTY_HINT_NONE + var prop_hint_string: String = "" for prop in node.get_property_list(): if prop.name == property: found = true prop_type = prop.get("type", TYPE_NIL) + prop_hint = prop.get("hint", PROPERTY_HINT_NONE) + prop_hint_string = prop.get("hint_string", "") break if not found: return ErrorCodes.make(ErrorCodes.PROPERTY_NOT_ON_CLASS, McpPropertyErrors.build_message(node, property)) @@ -230,7 +244,45 @@ func set_property(params: Dictionary) -> Dictionary: var nil_resource_string: bool = target_type == TYPE_NIL and (value == "" or (value is String and value.begins_with("res://"))) var resource_string_value: bool = value is String and (target_type == TYPE_OBJECT or nil_resource_string) - if resource_string_value: + ## Node-typed exports (`@export var target: Control`) are TYPE_OBJECT with + ## PROPERTY_HINT_NODE_TYPE, so a string here is a node path, not a + ## resource path (#1144): a leading "/" is a clean scene path like `path`; + ## anything else is relative to the node, which is how the Inspector + ## stores the reference. "" still clears through the branch below. + var node_typed_path: bool = ( + target_type == TYPE_OBJECT + and prop_hint == PROPERTY_HINT_NODE_TYPE + and value is String + and not (value as String).is_empty() + ) + if node_typed_path: + var target: Node = _resolve_node_value(value, node, scene_root) + if target == null: + return ErrorCodes.make( + ErrorCodes.NODE_NOT_FOUND, + "value: no node at %s. Paths starting with \"/\" are scene paths (e.g. \"/%s/Player\"); others are relative to %s (e.g. \"../Player\")." % [ + value, scene_root.name, node_path, + ], + ) + ## An absolute editor-tree path ("/root", "/root/@EditorNode@/...") or + ## a relative path climbing past the root resolves to a node outside + ## the edited scene; it would commit but can never be saved with it. + if target != scene_root and not scene_root.is_ancestor_of(target): + return ErrorCodes.make( + ErrorCodes.INVALID_PARAMS, + "value: %s resolves to a node outside the edited scene; the target must be \"/%s\" or one of its descendants" % [ + value, scene_root.name, + ], + ) + if not _node_matches_hint(target, prop_hint_string): + return ErrorCodes.make( + ErrorCodes.WRONG_TYPE, + "value: %s is a %s, which is not a %s" % [ + value, _object_type_label(target.get_class(), target.get_script()), prop_hint_string, + ], + ) + value = target + elif resource_string_value: if value == "": value = null else: @@ -305,13 +357,56 @@ func set_property(params: Dictionary) -> Dictionary: "data": { "path": node_path, "property": property, - "value": _serialize_value(node.get(property)), - "old_value": _serialize_value(old_value), + "value": _serialize_slot(node.get(property), scene_root), + "old_value": _serialize_slot(old_value, scene_root), "undoable": true, } } +## Resolve the string value of a Node-typed property (#1144): a leading "/" +## is a clean scene path with the same forms `path` accepts; anything else +## is a NodePath relative to `node`. +static func _resolve_node_value(value: String, node: Node, scene_root: Node) -> Node: + if value.begins_with("/"): + return McpScenePath.resolve(value, scene_root) + return node.get_node_or_null(NodePath(value)) + + +## Does `target` satisfy a PROPERTY_HINT_NODE_TYPE hint string? The hint is +## the exported type's name — a ClassDB class ("Control") or a script +## class_name ("Player") — and may list several, comma-separated. An empty +## hint (a plain `Node` export on some engine versions) accepts any node. +static func _node_matches_hint(target: Node, hint_string: String) -> bool: + var any_named := false + for hint in hint_string.split(","): + var wanted := hint.strip_edges() + if wanted.is_empty(): + continue + any_named = true + if ClassDB.class_exists(wanted): + if target.is_class(wanted): + return true + continue + var script: Variant = target.get_script() + while script is Script: + if String((script as Script).get_global_name()) == wanted: + return true + script = (script as Script).get_base_script() + return not any_named + + +## set_property's response value: a Node lands as its clean scene path so the +## agent sees "/Main/Player", not Godot's object repr (#1144); everything +## else serializes as before. +static func _serialize_slot(value: Variant, scene_root: Node) -> Variant: + if value is Node: + var clean := McpScenePath.from_node(value, scene_root) + if not clean.is_empty(): + return clean + return _serialize_value(value) + + func rename_node(params: Dictionary) -> Dictionary: var resolved := _resolve_node(params) if resolved.has("error"): @@ -1322,7 +1417,7 @@ func get_node_properties(params: Dictionary) -> Dictionary: properties.append({ "name": prop.name, "type": type_string(prop.type), - "value": _serialize_value(node.get(prop.name)), + "value": _serialize_slot(node.get(prop.name), scene_root), }) # Requested names that matched no editor-visible property — distinguishes # "you asked for something that doesn't exist" from "exists and is null". diff --git a/addons/godot_ai/mcp_dock.gd b/addons/godot_ai/mcp_dock.gd index 5157457..1c63383 100644 --- a/addons/godot_ai/mcp_dock.gd +++ b/addons/godot_ai/mcp_dock.gd @@ -167,11 +167,6 @@ var _uv_recheck_pending := false ## (foreign-port branch never sets `_server_pid`, so `_stop_server` ## can't kill it); the line has to show the mismatch honestly. var _setup_server_label: Label -## Last rendered server-version string. `_update_status_label` runs every -## frame; early-outs text repaint when nothing changed. Empty means -## "no line rendered yet" (dev-checkout branch doesn't render a -## user-mode Server line). -var _last_rendered_server_text: String = "" ## Restart-server button shown next to the Setup container when ## `McpConnection.server_version` drifts from the plugin version. Hidden ## in the match case so the UI stays calm. @@ -1358,13 +1353,19 @@ func _refresh_server_version_label(server_status: Dictionary = {}) -> void: ## look like it had multiple restart paths. or (is_incompatible and can_recover and _crash_restart_btn == null) ) - if text == _last_rendered_server_text: - _setup_server_label.add_theme_color_override("font_color", color) - _update_restart_button(show_restart) - return - _last_rendered_server_text = text + ## Runs every frame, so write only what changed (#1121). `Label.text` + ## already ignores same-value writes; `add_theme_color_override` does + ## not — every call re-sends NOTIFICATION_THEME_CHANGED and queues a + ## redraw, which kept the idle editor redrawing while this row was + ## visible. Compare against the label itself so a rebuilt row still + ## gets its color and a same-text state change (amber drift -> red + ## incompatible) still repaints. _setup_server_label.text = text - _setup_server_label.add_theme_color_override("font_color", color) + if ( + not _setup_server_label.has_theme_color_override("font_color") + or _setup_server_label.get_theme_color("font_color") != color + ): + _setup_server_label.add_theme_color_override("font_color", color) _update_restart_button(show_restart) @@ -1382,12 +1383,10 @@ func _on_restart_stale_server() -> void: if _server_restart_in_progress: return _server_restart_in_progress = true - _last_rendered_server_text = "" _refresh_server_version_label() if not is_inside_tree(): _dispatch_stale_server_restart() _server_restart_in_progress = false - _last_rendered_server_text = "" _refresh_server_version_label() return call_deferred("_restart_stale_server_after_feedback") @@ -1397,7 +1396,6 @@ func _restart_stale_server_after_feedback() -> void: await get_tree().create_timer(0.15).timeout if not _dispatch_stale_server_restart(): _server_restart_in_progress = false - _last_rendered_server_text = "" _refresh_server_version_label() @@ -1418,7 +1416,6 @@ func present_lifecycle_action_result(accepted: bool) -> void: if accepted: return _server_restart_in_progress = false - _last_rendered_server_text = "" _refresh_server_version_label() @@ -1516,7 +1513,6 @@ func _refresh_setup_status() -> void: _version_restart_btn.visible = false server_row.add_child(_version_restart_btn) _setup_container.add_child(server_row) - _last_rendered_server_text = "" _refresh_server_version_label() else: _setup_container.add_child(_make_status_row("uv", "not found", Color.RED)) diff --git a/addons/godot_ai/plugin.cfg b/addons/godot_ai/plugin.cfg index 15856ad..479f7dc 100644 --- a/addons/godot_ai/plugin.cfg +++ b/addons/godot_ai/plugin.cfg @@ -3,5 +3,5 @@ name="Godot AI" description="MCP server and AI tools for Godot" author="Godot AI" -version="4.2.3" +version="4.3.0" script="plugin.gd" diff --git a/addons/godot_ai/runtime/game_helper.gd b/addons/godot_ai/runtime/game_helper.gd index 1859fb3..cfd8b83 100644 --- a/addons/godot_ai/runtime/game_helper.gd +++ b/addons/godot_ai/runtime/game_helper.gd @@ -156,8 +156,13 @@ func _process(_delta: float) -> void: ## through the debugger packet path in a single tick. Surplus stays in ## `_pending_outbound` and bleeds out across subsequent frames. if not _logger_attached or _logger == null: + _pending_outbound.clear() return if not EngineDebugger.is_active(): + ## No remote can consume these lines. Drop current and already-drained + ## batches so a detached/headless run cannot retain logs indefinitely (#1123). + _logger.clear() + _pending_outbound.clear() return if _pending_outbound.is_empty(): if not _logger.has_pending(): @@ -1024,13 +1029,7 @@ func _handle_eval(data: Array) -> void: _eval_token_counter += 1 var token := str(_eval_token_counter) var run_fn := "_mcp_run_%s" % token - var script_source := ( - "extends Node\n" - + "func execute():\n" - + "\treturn await %s()\n\n" % run_fn - + "func %s():\n" % run_fn - + _indent_eval_code(code) - ) + var script_source := _build_eval_script_source(run_fn, code) ## Snapshot the logger's script-error seq BEFORE running so we only attribute ## errors raised by this eval. In a debug build a parse error aborts reload() @@ -1227,7 +1226,23 @@ func _handle_eval_check(data: Array) -> void: _try_report_eval_runtime_error(request_id) -func _indent_eval_code(code: String) -> String: +## `execute()` always returns, so it carries a return type. The inner function +## holds caller code that need not return on every path; a declared return type +## there makes Godot reject it with "Not all code paths return a value". It +## stays untyped and suppresses `untyped_declaration` on its own declaration +## instead (#1119). The annotation shares the `func` line so the line numbers +## reported for caller code do not move. +static func _build_eval_script_source(run_fn: String, code: String) -> String: + return ( + "extends Node\n" + + "func execute() -> Variant:\n" + + "\treturn await %s()\n\n" % run_fn + + "@warning_ignore(\"untyped_declaration\") func %s():\n" % run_fn + + _indent_eval_code(code) + ) + + +static func _indent_eval_code(code: String) -> String: var lines: PackedStringArray = code.split("\n") var out := "" for line in lines: diff --git a/addons/godot_ai/runtime/game_logger.gd b/addons/godot_ai/runtime/game_logger.gd index 6384960..30f49f7 100644 --- a/addons/godot_ai/runtime/game_logger.gd +++ b/addons/godot_ai/runtime/game_logger.gd @@ -37,6 +37,11 @@ var _mutex := Mutex.new() ## (types 0/1) never count. Mutex-guarded: _log_error can fire from any thread. const _ERROR_TYPE_SCRIPT := 2 const _MAX_RECENT_SCRIPT_ERRORS := 64 +const _MAX_PENDING := 4096 +const _PENDING_TRIM_THRESHOLD := _MAX_PENDING * 2 +## Lines trimmed from `_pending` since the last drain. The next drain reports +## the count as a leading warn entry, so a gap in the log is never silent. +var _dropped: int = 0 var _script_error_seq: int = 0 var _recent_script_errors: Array = [] @@ -93,6 +98,19 @@ func _append(level: String, text: String, details: Dictionary = {}) -> void: _pending.append([level, text]) else: _pending.append([level, text, details.duplicate(true)]) + ## Drop an old batch when a script floods the logger before the next frame. + ## Trimming at 2x the cap amortizes the slice instead of doing it per line. + if _pending.size() >= _PENDING_TRIM_THRESHOLD: + _dropped += _pending.size() - _MAX_PENDING + _pending = _pending.slice(_pending.size() - _MAX_PENDING) + _mutex.unlock() + + +## Drop queued lines that no active debugger can consume. +func clear() -> void: + _mutex.lock() + _pending.clear() + _dropped = 0 _mutex.unlock() @@ -102,7 +120,15 @@ func drain() -> Array: _mutex.lock() var out := _pending _pending = [] + var dropped := _dropped + _dropped = 0 _mutex.unlock() + if dropped > 0: + out.push_front([ + "warn", + "[godot_ai] dropped %d older game log lines: the game logged faster than they could be forwarded" + % dropped, + ]) return out diff --git a/addons/godot_ai/utils/client_mutation_lock.gd b/addons/godot_ai/utils/client_mutation_lock.gd index 864f8cf..583a902 100644 --- a/addons/godot_ai/utils/client_mutation_lock.gd +++ b/addons/godot_ai/utils/client_mutation_lock.gd @@ -102,6 +102,14 @@ static func _root_path() -> String: ## Unlike user://, the OS config directory is shared by every Godot ## project owned by this account. User-scope client config is shared too, ## so a project-specific lock would leave the real cross-editor race open. + ## + ## A Flatpak editor that shares the home directory writes the host's client + ## configs, the same files an editor outside the sandbox writes, while its + ## own OS config directory is the per-app ~/.var/app//config. It takes + ## the lock where those other editors look for it. + var host_config_home := McpPathTemplate.flatpak_host_config_home() + if not host_config_home.is_empty(): + return host_config_home.path_join(ROOT_DIR_NAME) return OS.get_config_dir().path_join(ROOT_DIR_NAME) diff --git a/addons/godot_ai/utils/error_codes.gd b/addons/godot_ai/utils/error_codes.gd index 9136144..460696c 100644 --- a/addons/godot_ai/utils/error_codes.gd +++ b/addons/godot_ai/utils/error_codes.gd @@ -26,6 +26,11 @@ const EDITOR_NOT_READY := "EDITOR_NOT_READY" const UNKNOWN_COMMAND := "UNKNOWN_COMMAND" const INTERNAL_ERROR := "INTERNAL_ERROR" const DEFERRED_TIMEOUT := "DEFERRED_TIMEOUT" +## #1120: filesystem mutation discovery failed before any disk effect +## (deadline/cancellation, entry/byte limits, or unreadable owner metadata). +## Keep this separate from INVALID_PARAMS so callers can distinguish an +## environmental discovery failure from a bad filesystem_manage request. +const FILESYSTEM_DISCOVERY_FAILED := "FILESYSTEM_DISCOVERY_FAILED" ## Python-originated transport/attach bridge codes. GDScript has no emit path, ## but the public registry intentionally mirrors protocol/errors.py. const TRANSPORT_OUTCOME_UNKNOWN := "TRANSPORT_OUTCOME_UNKNOWN" diff --git a/addons/godot_ai/utils/port_resolver.gd b/addons/godot_ai/utils/port_resolver.gd index f5a72b5..edd0d6d 100644 --- a/addons/godot_ai/utils/port_resolver.gd +++ b/addons/godot_ai/utils/port_resolver.gd @@ -21,6 +21,12 @@ const SNAPSHOT_DIAGNOSTIC_CATEGORIES := [ "snapshot_size", "snapshot_json", "snapshot_shape", "collector_null", "row_shape", "row_pid", "row_chain", "row_identity", "lineage_cycle", ] +## Public lifecycle evidence uses a smaller, stable vocabulary than the +## internal collector diagnostics. Keep the detailed category internally so a +## future fix can distinguish malformed JSON from an inaccessible ancestor. +const SNAPSHOT_FAILURE_CATEGORIES := [ + "process_query", "json_shape", "ancestor_capture", "identity_mismatch", +] static var _process_spawn_mutex := Mutex.new() enum PortOccupancy { UNKNOWN, FREE, OCCUPIED } @@ -531,6 +537,39 @@ static func _snapshot_failure( return {"capture_error": true} +## Map one internal collector refusal to the bounded category retained in the +## editor log. `row_identity` at depth zero is malformed target metadata; +## the same refusal on an ancestor is an ancestor-capture failure. A refusal +## this table does not know stays "unknown" instead of borrowing a category. +static func snapshot_failure_category(detail: String, depth := -1) -> String: + match detail: + "shell_exit", "empty_output", "invalid_pid", "collector_null": + return "process_query" + "lineage_cycle": + return "ancestor_capture" + "row_identity": + return "ancestor_capture" if depth > 0 else "json_shape" + "outer_size", "outer_json", "outer_shape", "snapshot_size", "snapshot_json", \ + "snapshot_shape", "row_shape", "row_pid", "row_chain": + return "json_shape" + return "unknown" + + +## Return only the creation-time portion of a captured identity. Windows rows +## append the command line after `|`; Linux rows use `linux:|`. +## The command line is intentionally never copied into diagnostics. +static func process_creation_identity(pid: int, snapshot: Variant = null) -> String: + if pid <= 1 or not (snapshot is Dictionary) or capture_failed(snapshot): + return "" + var identity := str(_process_snapshot_row(snapshot, pid).get("identity", "")).strip_edges() + if identity.is_empty(): + return "" + var separator := identity.find("|") + if separator > 0: + identity = identity.substr(0, separator).strip_edges() + return identity.substr(0, 128) + + ## An unavailable capture is not evidence that its PID has exited. static func capture_failed(snapshot: Variant) -> bool: diff --git a/addons/godot_ai/utils/server_lifecycle.gd b/addons/godot_ai/utils/server_lifecycle.gd index a46bb2b..85dc2ef 100644 --- a/addons/godot_ai/utils/server_lifecycle.gd +++ b/addons/godot_ai/utils/server_lifecycle.gd @@ -51,6 +51,7 @@ const STALE_PRE_V4_HINT := "stale_pre_v4_server" const DEFAULT_PROVE_TIMEOUT_MS := 180_000 const LAUNCH_FINGERPRINT_TIMEOUT_MS := 15_000 const REPLACEMENT_TTL_MS := 15_000 +const IDENTITY_CAPTURE_STAGES := ["launch", "first_server", "final_server"] ## An authenticated endpoint that drops (a late keepalive, a server restart) ## is re-probed automatically with this backoff, each attempt re-proving the ## server exactly as a start does; after the last one the block stays until @@ -126,6 +127,8 @@ func _begin_start_episode(existing_id := 0, probe := true) -> void: "blocked_target": {}, "launch": {}, "after_stop": "", + "snapshot_diagnostic": {}, + "proof_stage_attempts": {"launch": 0, "first_server": 0, "final_server": 0}, } _publish() if not probe: @@ -461,6 +464,7 @@ func _complete_launch(result: Dictionary) -> void: func _complete_prove(result: Dictionary) -> void: + _remember_snapshot_evidence(result) if bool(result.get("pending", false)): _episode["proof_pending_reason"] = str(result.get("reason", "unknown")) if Time.get_ticks_msec() >= int(_episode.get("prove_deadline_msec", 0)): @@ -468,6 +472,7 @@ func _complete_prove(result: Dictionary) -> void: "proof_timeout", "The managed server proof timed out at %s." % str(_episode.get("proof_pending_reason", "unknown")) + + _snapshot_diagnostic_sentence() + startup_report_summary(str(_plan.get("startup_report", ""))), ) return @@ -478,6 +483,7 @@ func _complete_prove(result: Dictionary) -> void: var pending_reason := str(_episode.get("proof_pending_reason", "")) if not pending_reason.is_empty(): message += " Last pending proof: %s." % pending_reason + message += _snapshot_diagnostic_sentence() _block(str(result.get("reason", "proof_failed")), message) return var launch: Dictionary = _episode.get("launch", {}) @@ -485,7 +491,11 @@ func _complete_prove(result: Dictionary) -> void: var fingerprint := str(result.get("fingerprint", "")) var exact_grant = _owned_process_grant(pid, fingerprint) if not exact_grant.is_valid(): - _block("process_proof_failed", "The server process identity changed before proof completed.") + _block( + "process_proof_failed", + "The server process identity changed before proof completed." + + _snapshot_diagnostic_sentence(), + ) return _process_grant = exact_grant launch["pid"] = pid @@ -861,8 +871,9 @@ func _effect_launch(payload: Dictionary) -> Dictionary: "ok": not fingerprint.is_empty(), "reason": "launch_unproven" if fingerprint.is_empty() else "", "message": ( - _launch_unproven_message(pid, attempts, Time.get_ticks_msec() - capture_started) - + _snapshot_diagnostic_summary(snapshot_diagnostics) + _launch_unproven_message( + pid, attempts, Time.get_ticks_msec() - capture_started, snapshot_diagnostics + ) if fingerprint.is_empty() else "" ), @@ -896,12 +907,13 @@ static func _snapshot_diagnostic_summary(diagnostics: Array) -> String: ## (#988, #1012 both surfaced as this bare sentence). Diagnostic only: the ## values are read once more after the capture budget and never grant ## authority. -func _launch_unproven_message(pid: int, attempts: Array, elapsed_ms: int) -> String: - var snapshot: Variant = _capture_process_snapshot(pid) +func _launch_unproven_message( + pid: int, attempts: Array, elapsed_ms: int, snapshot_diagnostics: Array = [] +) -> String: + var capture_diagnostics: Array = [] + var snapshot: Variant = _capture_process_snapshot(pid, capture_diagnostics) var alive := PortResolver.pid_alive(pid, snapshot) - var commandline := PortResolver.process_commandline(pid, snapshot) if alive else "" - if commandline.length() > 200: - commandline = commandline.substr(0, 199) + "…" + var creation_identity := PortResolver.process_creation_identity(pid, snapshot) ## Summarise the per-attempt refusals as "reason×count" in first-seen order. var counts := {} var order: Array[String] = [] @@ -914,43 +926,218 @@ func _launch_unproven_message(pid: int, attempts: Array, elapsed_ms: int) -> Str var summary: Array[String] = [] for key in order: summary.append("%s×%d" % [key, int(counts[key])]) - return ( + var message := ( "The launched process identity could not be captured in %d attempts over %.1f s " - + "(pid %d, now alive=%s, refusals: %s%s)." + + "(pid %d, creation_identity=%s, now alive=%s, refusals: %s)." ) % [ attempts.size(), elapsed_ms / 1000.0, pid, + "unavailable" if creation_identity.is_empty() else creation_identity, "unknown" if PortResolver.capture_failed(snapshot) else ("yes" if alive else "no"), ", ".join(summary) if not summary.is_empty() else "none recorded", - "" if commandline.is_empty() else "; command: " + commandline, + ] + return message + _snapshot_diagnostic_summary(snapshot_diagnostics + capture_diagnostics) + + +func _capture_process_snapshot(pid: int, diagnostics: Variant = null) -> Variant: + return PortResolver.capture_process_snapshot(pid, diagnostics) + + +func _remember_snapshot_evidence(result: Dictionary) -> void: + var diagnostic: Variant = result.get("snapshot_diagnostic") + if diagnostic is Dictionary: + _episode["snapshot_diagnostic"] = diagnostic.duplicate(true) + var attempts: Variant = result.get("proof_stage_attempts") + if attempts is Dictionary: + _episode["proof_stage_attempts"] = attempts.duplicate(true) + + +func _snapshot_diagnostic_sentence() -> String: + var diagnostic: Variant = _episode.get("snapshot_diagnostic", {}) + if not (diagnostic is Dictionary) or diagnostic.is_empty(): + return "" + var stage := str(diagnostic.get("stage", "")) + if stage not in IDENTITY_CAPTURE_STAGES: + stage = "unknown" + var category := str(diagnostic.get("category", "")) + if category not in PortResolver.SNAPSHOT_FAILURE_CATEGORIES: + category = "unknown" + var identity := str(diagnostic.get("creation_identity", "")).strip_edges() + if identity.is_empty(): + identity = "unavailable" + identity = identity.get_slice("|", 0).replace("\n", " ").replace("\r", " ").substr(0, 128) + ## The collector's own refusal, when one was recorded: the fixed category + ## alone cannot tell a shell that exited from one that printed nothing. + var detail := str(diagnostic.get("detail", "")) + if not detail.is_empty(): + if detail not in PortResolver.SNAPSHOT_DIAGNOSTIC_CATEGORIES: + detail = "unknown" + category += " detail=%s" % detail + var depth := int(diagnostic.get("depth", -1)) + if depth >= 0: + category += " depth=%d" % clampi(depth, 0, 16) + return ( + " Identity capture: stage=%s category=%s pid=%d creation_identity=%s attempt=%d elapsed_ms=%d." + ) % [ + stage, + category, + int(diagnostic.get("pid", -1)), + identity, + clampi(int(diagnostic.get("attempt", 1)), 1, 100000), + clampi(int(diagnostic.get("elapsed_ms", 0)), 0, 600000), ] -func _capture_process_snapshot(pid: int) -> Variant: - return PortResolver.capture_process_snapshot(pid) +func _capture_identity( + stage: String, pid: int, stage_attempts: Dictionary +) -> Dictionary: + var attempt := int(stage_attempts.get(stage, 0)) + 1 + stage_attempts[stage] = attempt + var diagnostics: Array = [] + var began := Time.get_ticks_msec() + var snapshot: Variant = _capture_process_snapshot(pid, diagnostics) + return { + "snapshot": snapshot, + "diagnostic": _snapshot_evidence( + stage, pid, snapshot, diagnostics, attempt, Time.get_ticks_msec() - began + ), + } + + +func _record_snapshot_capture( + stage: String, pid: int, snapshot: Variant, stage_attempts: Dictionary, elapsed_ms: int = 0 +) -> Dictionary: + var attempt := int(stage_attempts.get(stage, 0)) + 1 + stage_attempts[stage] = attempt + return { + "snapshot": snapshot, + "diagnostic": _snapshot_evidence(stage, pid, snapshot, [], attempt, elapsed_ms), + } + + +static func _capture_elapsed_ms(capture: Dictionary) -> int: + var diagnostic: Variant = capture.get("diagnostic", {}) + return int(diagnostic.get("elapsed_ms", 0)) if diagnostic is Dictionary else 0 + + +static func _retarget_snapshot_capture( + capture: Dictionary, stage: String, pid: int +) -> Dictionary: + var snapshot: Variant = capture.get("snapshot") + var old_diagnostic: Variant = capture.get("diagnostic", {}) + var attempt := int(old_diagnostic.get("attempt", 1)) if old_diagnostic is Dictionary else 1 + var elapsed_ms := int(old_diagnostic.get("elapsed_ms", 0)) if old_diagnostic is Dictionary else 0 + return { + "snapshot": snapshot, + "diagnostic": _snapshot_evidence(stage, pid, snapshot, [], attempt, elapsed_ms), + } + + +static func _snapshot_evidence( + stage: String, + pid: int, + snapshot: Variant, + diagnostics: Array, + attempt: int, + elapsed_ms: int, + category_override := "", +) -> Dictionary: + var detail := "" + var depth := -1 + if not diagnostics.is_empty(): + var last: Variant = diagnostics[diagnostics.size() - 1] + if last is Dictionary: + detail = str(last.get("category", "")) + depth = int(last.get("depth", -1)) + var category := str(category_override) + if category.is_empty() and not detail.is_empty(): + category = PortResolver.snapshot_failure_category(detail, depth) + elif category.is_empty() and PortResolver.capture_failed(snapshot): + category = "process_query" + var creation_identity := PortResolver.process_creation_identity(pid, snapshot) + return { + "stage": stage if stage in IDENTITY_CAPTURE_STAGES else "unknown", + "category": ( + category + if category.is_empty() or category in PortResolver.SNAPSHOT_FAILURE_CATEGORIES + else "unknown" + ), + "detail": ( + detail + if detail.is_empty() or detail in PortResolver.SNAPSHOT_DIAGNOSTIC_CATEGORIES + else "unknown" + ), + "depth": clampi(depth, -1, 16), + "pid": pid, + "creation_identity": "unavailable" if creation_identity.is_empty() else creation_identity, + "attempt": clampi(attempt, 1, 100000), + "elapsed_ms": clampi(elapsed_ms, 0, 600000), + } + + +func _proof_pending( + reason: String, stage_attempts: Dictionary, diagnostic: Dictionary = {} +) -> Dictionary: + var result := { + "pending": true, + "reason": reason, + "proof_stage_attempts": stage_attempts.duplicate(true), + } + if not diagnostic.is_empty(): + result["snapshot_diagnostic"] = diagnostic.duplicate(true) + return result + + +func _capture_failure( + stage_attempts: Dictionary, capture: Dictionary, category_override := "" +) -> Dictionary: + var diagnostic: Dictionary = capture.get("diagnostic", {}).duplicate(true) + if not category_override.is_empty(): + diagnostic["category"] = category_override + return _proof_pending("identity_unavailable", stage_attempts, diagnostic) + + +func _identity_mismatch( + reason: String, stage_attempts: Dictionary, capture: Dictionary +) -> Dictionary: + var diagnostic: Dictionary = capture.get("diagnostic", {}).duplicate(true) + diagnostic["category"] = "identity_mismatch" + return _proof_pending(reason, stage_attempts, diagnostic) func _effect_prove(payload: Dictionary) -> Dictionary: var launch_grant = payload.get("grant") var launch_pid := int(launch_grant.process_id()) if launch_grant != null else -1 + var stage_attempts: Variant = payload.get("proof_stage_attempts", {}) + if not (stage_attempts is Dictionary): + stage_attempts = {} + else: + stage_attempts = stage_attempts.duplicate(true) + for stage in IDENTITY_CAPTURE_STAGES: + stage_attempts[stage] = maxi(0, int(stage_attempts.get(stage, 0))) ## The pid file is an untrusted hint. Reuse its ancestor snapshot only ## when it includes the launcher; the original PID read and all final ## capability, listener, identity, and lineage checks still follow. var hinted_pid := -1 var first_snapshot: Variant = null var launch_snapshot: Variant = null + var launch_capture: Dictionary = {} + var first_capture: Dictionary = {} if OS.get_name() == "Windows" and launch_pid > 1: hinted_pid = PortResolver.read_pid_file(str(payload.get("pid_file", ""))) if hinted_pid > 1: - first_snapshot = _capture_process_snapshot(hinted_pid) + first_capture = _capture_identity("launch", hinted_pid, stage_attempts) + first_snapshot = first_capture.get("snapshot") if PortResolver.process_descends_from(hinted_pid, launch_pid, first_snapshot): + launch_capture = _retarget_snapshot_capture(first_capture, "launch", launch_pid) launch_snapshot = first_snapshot if launch_snapshot == null: - launch_snapshot = _capture_process_snapshot(launch_pid) + launch_capture = _capture_identity("launch", launch_pid, stage_attempts) + launch_snapshot = launch_capture.get("snapshot") first_snapshot = null if PortResolver.capture_failed(launch_snapshot): - return {"pending": true, "reason": "identity_unavailable"} + return _capture_failure(stage_attempts, launch_capture) var launch_alive := PortResolver.pid_alive(launch_pid, launch_snapshot) var launch_fingerprint := ( PortResolver.process_fingerprint(launch_pid, launch_snapshot) @@ -963,72 +1150,103 @@ func _effect_prove(payload: Dictionary) -> Dictionary: launch_fingerprint, ) if launch_disposition != "owned": - return { - "ok": false, - "reason": "launch_%s" % launch_disposition, - "message": ( - "The launched server process exited or changed identity before publishing capabilities." - + startup_report_summary(str(payload.get("startup_report", ""))) - ), - } + if launch_disposition == "gone": + return { + "ok": false, + "reason": "launch_gone", + "message": ( + "The launched server process exited or changed identity before publishing capabilities." + + startup_report_summary(str(payload.get("startup_report", ""))) + ), + "proof_stage_attempts": stage_attempts.duplicate(true), + } + var mismatch := _identity_mismatch("launch_%s" % launch_disposition, stage_attempts, launch_capture) + mismatch.erase("pending") + mismatch["ok"] = false + mismatch["message"] = ( + "The launched server process exited or changed identity before publishing capabilities." + + startup_report_summary(str(payload.get("startup_report", ""))) + ) + return mismatch var port := int(payload.http_port) var capability := _read_capability(port) if capability.is_empty() or str(capability.get("instance_nonce", "")) == str(payload.get("baseline_instance_id", "")): - return {"pending": true, "reason": "capability_record"} + return _proof_pending("capability_record", stage_attempts) if ( str(capability.get("http", "")) != str(payload.get("http_capability", "")) or str(capability.get("websocket", "")) != str(payload.get("ws_capability", "")) ): - return {"pending": true, "reason": "capability_pair"} + return _proof_pending("capability_pair", stage_attempts) var listeners := PortResolver.windows_listener_snapshot() if OS.get_name() == "Windows" else {} var live := _probe_with_capability(port, capability, int(payload.timeout_ms), listeners) if not _authenticated_status_matches_record(live, capability): - return {"pending": true, "reason": "authenticated_status"} + return _proof_pending("authenticated_status", stage_attempts) var version := str(live.get("version", "")) var ws_port := int(live.get("ws_port", 0)) if not _server_status_compatibility( version, str(payload.expected_version), ws_port, int(payload.expected_ws_port) ).get("compatible", false): - return {"ok": false, "reason": "incompatible", "message": "The launched server does not match this plugin."} + return { + "ok": false, + "reason": "incompatible", + "message": "The launched server does not match this plugin.", + "proof_stage_attempts": stage_attempts.duplicate(true), + } var pid := PortResolver.read_pid_file(str(payload.get("pid_file", ""))) if pid <= 1: - return {"pending": true, "reason": "pid_file"} + return _proof_pending("pid_file", stage_attempts) if not PortResolver.find_all_pids_on_port(port, Callable(), listeners).has(pid): - return {"pending": true, "reason": "listener_pid"} + return _proof_pending("listener_pid", stage_attempts) if first_snapshot == null or pid != hinted_pid: - first_snapshot = ( - launch_snapshot if pid == launch_pid else _capture_process_snapshot(pid) + if pid == launch_pid: + first_snapshot = launch_snapshot + first_capture = _record_snapshot_capture( + "first_server", pid, first_snapshot, stage_attempts, + _capture_elapsed_ms(launch_capture) + ) + else: + first_capture = _capture_identity("first_server", pid, stage_attempts) + first_snapshot = first_capture.get("snapshot") + else: + first_capture = _record_snapshot_capture( + "first_server", pid, first_snapshot, stage_attempts, + _capture_elapsed_ms(first_capture) ) if PortResolver.capture_failed(first_snapshot): - return {"pending": true, "reason": "identity_unavailable"} + return _capture_failure(stage_attempts, first_capture) if not PortResolver.pid_cmdline_is_godot_ai(pid, first_snapshot): - return {"pending": true, "reason": "process_brand"} + return _identity_mismatch("process_brand", stage_attempts, first_capture) if not PortResolver.process_descends_from(pid, launch_pid, first_snapshot): - return {"pending": true, "reason": "launch_lineage"} + return _identity_mismatch("launch_lineage", stage_attempts, first_capture) var fingerprint := PortResolver.process_fingerprint(pid, first_snapshot) if fingerprint.is_empty(): - return {"pending": true, "reason": "process_fingerprint"} + return _identity_mismatch("process_fingerprint", stage_attempts, first_capture) ## Close every capture window before minting process authority. The same ## launch-lineage PID must still own the listener, the private capability ## record must still name the authenticated instance, and the process ## fingerprint must remain unchanged after the final probe. var final_capability := _read_capability(port) var final_live := _probe_with_capability(port, final_capability, int(payload.timeout_ms), listeners) - var final_snapshot: Variant = _capture_process_snapshot(pid) + var final_capture := _capture_identity("final_server", pid, stage_attempts) + var final_snapshot: Variant = final_capture.get("snapshot") if PortResolver.capture_failed(final_snapshot): - return {"pending": true, "reason": "identity_unavailable"} + return _capture_failure(stage_attempts, final_capture) + var final_fingerprint := PortResolver.process_fingerprint(pid, final_snapshot) var final_listeners := PortResolver.windows_listener_snapshot() if OS.get_name() == "Windows" else {} if ( PortResolver.read_pid_file(str(payload.get("pid_file", ""))) != pid or not PortResolver.find_all_pids_on_port(port, Callable(), final_listeners).has(pid) or not PortResolver.process_descends_from(pid, launch_pid, final_snapshot) or not PortResolver.pid_cmdline_is_godot_ai(pid, final_snapshot) - or PortResolver.process_fingerprint(pid, final_snapshot) != fingerprint - or final_capability != capability + or final_fingerprint != fingerprint + ): + return _identity_mismatch("final_capture_window", stage_attempts, final_capture) + if ( + final_capability != capability or not _authenticated_status_matches_record(final_live, final_capability) or str(final_live.get("instance_id", "")) != str(live.get("instance_id", "")) ): - return {"pending": true, "reason": "final_capture_window"} + return _proof_pending("final_capture_window", stage_attempts) var final_version := str(final_live.get("version", "")) var final_ws_port := int(final_live.get("ws_port", 0)) if not _server_status_compatibility( @@ -1041,6 +1259,7 @@ func _effect_prove(payload: Dictionary) -> Dictionary: "ok": false, "reason": "incompatible", "message": "The launched server changed compatibility before authority was granted.", + "proof_stage_attempts": stage_attempts.duplicate(true), } return { "ok": true, @@ -1048,6 +1267,7 @@ func _effect_prove(payload: Dictionary) -> Dictionary: "fingerprint": fingerprint, "version": final_version, "transport": _transport_from(port, final_ws_port, final_live, final_capability), + "proof_stage_attempts": stage_attempts.duplicate(true), } @@ -1289,6 +1509,9 @@ func _prove_payload() -> Dictionary: "ws_capability": str(launch.get("ws_capability", "")), "baseline_instance_id": str(launch.get("baseline_instance_id", "")), "grant": _process_grant, + "proof_stage_attempts": _episode.get( + "proof_stage_attempts", {"launch": 0, "first_server": 0, "final_server": 0} + ), } @@ -1742,7 +1965,8 @@ static func _dormant_episode(episode_id: int) -> Dictionary: "id": episode_id, "state": DORMANT, "phase": "", "reason": "", "message": "Server lifecycle is dormant.", "ready_kind": "", "effect": "", "expected_version": "", "actual_version": "", "blocked_target": {}, - "launch": {}, "after_stop": "", + "launch": {}, "after_stop": "", "snapshot_diagnostic": {}, + "proof_stage_attempts": {"launch": 0, "first_server": 0, "final_server": 0}, } diff --git a/addons/godot_ai/utils/transport_capability.gd b/addons/godot_ai/utils/transport_capability.gd index 69fcdc7..6e2e072 100644 --- a/addons/godot_ai/utils/transport_capability.gd +++ b/addons/godot_ai/utils/transport_capability.gd @@ -13,6 +13,19 @@ const _GROUP_OTHER_PERMISSION_MASK := 0x3f ## 0077 const _GROUP_OTHER_WRITE_MASK := 0x12 ## 0022 const _MAX_LINK_HOPS := 8 const _SYSTEM_TEMP_ROOTS: Array[String] = ["/tmp", "/private/tmp", "/var/tmp"] +const _FLATPAK_INFO_PATH := "/.flatpak-info" +## Every read-write spelling of the two grants that expose the home directory. +## Flatpak 1.16 writes plain read-write as the bare name; `:create` is +## read-write too. `:ro` is absent on purpose. +const _FLATPAK_HOME_GRANTS: Array[String] = [ + "host", "host:rw", "host:create", "home", "home:rw", "home:create", +] +## Root directories a `host` grant leaves out of the sandbox, where Flatpak's +## own runtime lives (`dont_mount_in_root` in flatpak-context.c). +const _FLATPAK_HOST_EXCLUDED: Array[String] = [ + "lib", "lib32", "lib64", "bin", "sbin", "usr", "boot", "root", + "tmp", "etc", "app", "run", "proc", "sys", "dev", "var", +] const _KEYS: Array[String] = [ "version", "http", "websocket", "instance_nonce", ] @@ -322,10 +335,371 @@ static func path_for_http_port(http_port: int) -> String: "Library/Application Support/godot-ai/capabilities" ) else: - directory = OS.get_environment("XDG_CONFIG_HOME").strip_edges() - if directory.is_empty(): - directory = OS.get_environment("HOME").path_join(".config") - directory = directory.path_join("godot-ai/capabilities") + directory = linux_config_home(_flatpak_info()).path_join("godot-ai/capabilities") if directory.is_empty() or not directory.is_absolute_path(): return "" return directory.simplify_path().path_join("http-%d.json" % http_port) + + +## The config directory a Linux process outside this one would also name. +## Flatpak points `XDG_CONFIG_HOME` at the app's own `~/.var/app//config`, +## which a client outside that sandbox never reads. When the sandbox shares +## the host's config directory, that is `HOST_XDG_CONFIG_HOME` if the host set +## one, else the `~/.config` default. Python's `capability_directory` applies +## the same rule, so the server publishes where this reads. +static func linux_config_home(flatpak_info: String) -> String: + var config := OS.get_environment(linux_config_home_variable(flatpak_info)).strip_edges() + if config.is_empty(): + config = OS.get_environment("HOME").path_join(".config") + return config + + +## The variable that names that directory; unset or empty means `~/.config`. +static func linux_config_home_variable(flatpak_info: String) -> String: + return ( + "HOST_XDG_CONFIG_HOME" if flatpak_shares_config_home(flatpak_info) else "XDG_CONFIG_HOME" + ) + + +## Whether `/.flatpak-info` text describes a sandbox that can write the host's +## config directory at its own path: one that shares the home, or holds a +## read-write grant for the whole of `xdg-config`. Flatpak mounts that grant at +## the host's path only and leaves `XDG_CONFIG_HOME` on the per-app directory. +## An `xdg-config/` grant does not count: Flatpak mounts it inside the +## per-app directory as well, so `XDG_CONFIG_HOME` already reaches it. +static func flatpak_shares_config_home(flatpak_info: String) -> bool: + return ( + flatpak_shares_home(flatpak_info) + or bool(_flatpak_filesystems(flatpak_info).get("xdg-config", false)) + ) + + +## Whether `/.flatpak-info` text describes a sandbox that can write the host's +## home directory. A read-write `host` or `home` grant exposes the real home at +## its own path; a `:ro` grant, a narrower one, or none leaves the app's private +## directories as the only ones a process outside the sandbox can also see. +static func flatpak_shares_home(flatpak_info: String) -> bool: + var group := "" + for line in flatpak_info.split("\n"): + if line.begins_with("["): + group = line.strip_edges() + elif group == "[Context]" and line.begins_with("filesystems="): + for grant in line.trim_prefix("filesystems=").strip_edges().split(";"): + if grant in _FLATPAK_HOME_GRANTS: + return true + return false + return false + + +## The grant in `/.flatpak-info` text that decides whether the sandbox shares +## `path` with the host, so that a process outside it finds the same file at +## the same location. Returned as {root, location, writable}: the directory or +## file the grant covers, the grant as Flatpak spells it (`home`, `~/dir`, +## `xdg-config/dir`), and whether it is read-write. {} when no grant covers +## the path. +## +## `home` and `host` share the home directory, except `~/.var/app`: there +## Flatpak hides every app's directory but the sandbox's own, whatever the +## grant, and shares that one with no grant at all. `host` also shares the +## root directories its runtime does not occupy. `~/dir` and `/dir` share that +## location, and `xdg-config[/dir]` the host's config directory, +## `host_config_home`. The narrowest grant decides, as the narrowest mount +## does. The other `xdg-` names share nothing here: where they lie depends on +## host settings the sandbox cannot read, so their paths fail closed. +## +## A grant says what Flatpak was asked for, not what it mounted. It skips a +## location that does not exist when the app starts, and `/.flatpak-info` +## leaves out denials (`--nofilesystem`). `flatpak_blocking_mount` checks the +## answer against the mounts. +static func flatpak_grant_over( + flatpak_info: String, path: String, home: String, host_config_home: String +) -> Dictionary: + var target := path.simplify_path() + if not target.is_absolute_path(): + return {} + var grants := _flatpak_filesystems(flatpak_info) + var found := {} + for location in grants: + var root := _flatpak_grant_path(str(location), home, host_config_home) + if root.is_empty() or not path_is_within(root, target): + continue + var covered := str(found.get("root", "")) + if root.length() > covered.length(): + found = {"root": root, "location": str(location), "writable": bool(grants[location])} + elif root == covered and found["writable"] and not grants[location]: + ## Two spellings of one location: Flatpak mounts whichever it + ## reaches last, so only agreement counts as writable. + found = {"root": root, "location": str(location), "writable": false} + var home_dir := home.simplify_path() + var top := target.get_slice("/", 1) + var wide := {} + if not home.is_empty() and path_is_within(home_dir, target): + var apps_dir := home_dir.path_join(".var/app") + var app_id := flatpak_application_id(flatpak_info) + if path_is_within(apps_dir, target): + if not app_id.is_empty() and path_is_within(apps_dir.path_join(app_id), target): + wide = { + "root": apps_dir.path_join(app_id), + "location": apps_dir.path_join(app_id), + "writable": true, + } + elif grants.has("home") or grants.has("host"): + ## Flatpak applies the wider of the two modes to the home directory, + ## so a read-write `home` is always enough for a path inside it. + wide = { + "root": home_dir, + "location": "home", + "writable": bool(grants.get("home", false)) or bool(grants.get("host", false)), + } + elif grants.has("host") and not top.is_empty() and top not in _FLATPAK_HOST_EXCLUDED: + wide = {"root": "/" + top, "location": "host", "writable": bool(grants["host"])} + if str(wide.get("root", "")).length() > str(found.get("root", "")).length(): + return wide + return found + + +## The sandboxed app's ID from `/.flatpak-info` text, or "" outside Flatpak. +static func flatpak_application_id(flatpak_info: String) -> String: + var group := "" + for line in flatpak_info.split("\n"): + if line.begins_with("["): + group = line.strip_edges() + elif group == "[Application]" and line.begins_with("name="): + return line.trim_prefix("name=").strip_edges() + return "" + + +## Whether `directory` is `path` or one of its ancestors. +static func path_is_within(directory: String, path: String) -> bool: + return path == directory or path.begins_with(directory.trim_suffix("/") + "/") + + +## Where a grant's location lies in the sandbox, or "" for one this cannot +## place (`host-os`, `xdg-documents`, ...). +static func _flatpak_grant_path(location: String, home: String, host_config_home: String) -> String: + var placed := "" + if location == "~" or location.begins_with("~/"): + placed = home.path_join(location.substr(2)) + elif location.begins_with("/"): + placed = _home_spelling(location, home) + elif not host_config_home.is_empty() and ( + location == "xdg-config" or location.begins_with("xdg-config/") + ): + placed = host_config_home + location.trim_prefix("xdg-config") + return placed.simplify_path() if placed.is_absolute_path() else "" + + +## `/home` and `/var/home` are one directory on ostree systems, and Flatpak +## mounts a grant at whichever is real, keeping the link. A grant spelled the +## way the home directory is not therefore reads as the one that is. +static func _home_spelling(path: String, home: String) -> String: + for spelling in [["/var/home/", "/home/"], ["/home/", "/var/home/"]]: + if home.begins_with(spelling[0]) and path.begins_with(spelling[1]): + return str(spelling[0]) + path.trim_prefix(spelling[1]) + return path + + +## The `[Context] filesystems=` grants of `/.flatpak-info` text as +## {location: writable}, a location being the grant without its mode: `host`, +## `~/dir`, `/dir`, `xdg-config/dir`. Plain and `:create` grants are writable. +## `:ro`, a denial (`!`) and a mode this does not know are not. +static func _flatpak_filesystems(flatpak_info: String) -> Dictionary: + var grants := {} + var group := "" + for line in flatpak_info.split("\n"): + if line.begins_with("["): + group = line.strip_edges() + elif group == "[Context]" and line.begins_with("filesystems="): + for entry in _key_file_list(line.trim_prefix("filesystems=").strip_edges()): + var grant := _flatpak_location_and_mode(entry.trim_prefix("!")) + ## `!host:reset` withdraws inherited grants. It names no location. + if grant[0].is_empty() or grant[1] == "reset": + continue + grants[grant[0]] = not entry.begins_with("!") and grant[1] in ["", "rw", "create"] + break + return grants + + +## A key file list value split on its unescaped `;`, with the key file's own +## escapes resolved. Flatpak's `\:` and `\\` reach the entry as `\:` and `\\`. +static func _key_file_list(value: String) -> PackedStringArray: + var entries := PackedStringArray() + var entry := "" + var index := 0 + while index < value.length(): + var character := value[index] + if character == "\\" and index + 1 < value.length(): + index += 1 + match value[index]: + "s": + entry += " " + "n": + entry += "\n" + "t": + entry += "\t" + "r": + entry += "\r" + _: + entry += value[index] + elif character == ";": + entries.append(entry) + entry = "" + else: + entry += character + index += 1 + if not entry.is_empty(): + entries.append(entry) + return entries + + +## One grant as [location, mode]. Flatpak escapes a `:` or `\` that belongs to +## the location, and the first bare `:` starts the mode (`ro`, `create`). +static func _flatpak_location_and_mode(entry: String) -> PackedStringArray: + var location := "" + var index := 0 + while index < entry.length(): + var character := entry[index] + if character == "\\" and index + 1 < entry.length(): + index += 1 + location += entry[index] + elif character == ":": + return PackedStringArray([location, entry.substr(index + 1)]) + else: + location += character + index += 1 + return PackedStringArray([location, ""]) + + +## `/proc/self/mountinfo` text as one {mount_point, root, fstype, read_only} +## per mount, in the kernel's order. A line that does not parse is left out. +static func parse_mountinfo(mountinfo: String) -> Array[Dictionary]: + var mounts: Array[Dictionary] = [] + for line in mountinfo.split("\n", false): + var fields := line.split(" ", false) + ## Optional fields end at a lone "-"; the filesystem type follows it. + var separator := fields.find("-", 6) + if separator < 0 or separator + 1 >= fields.size(): + continue + mounts.append({ + "mount_point": _unescape_mount_path(fields[4]), + "root": _unescape_mount_path(fields[3]), + "fstype": fields[separator + 1], + "read_only": fields[5].split(",").has("ro"), + }) + return mounts + + +## The mount that holds `path`: the one with the longest mount point at or +## above it and, among equals, the last listed, which is mounted on top. +## {} when none is. +static func mount_for_path(mounts: Array[Dictionary], path: String) -> Dictionary: + var target := path.simplify_path() + var found := {} + for mount in mounts: + var mount_point := str(mount["mount_point"]) + if ( + path_is_within(mount_point, target) + and mount_point.length() >= str(found.get("mount_point", "")).length() + ): + found = mount + return found + + +## The mount that contradicts a grant covering `path`: a file written there +## would stay inside the sandbox, or cannot be written. {} when the mounts +## bear the grant out. +## +## An ungranted location lies on the tmpfs Flatpak builds the sandbox on, and +## so does a granted one that did not exist when the app started. A denial +## hides a location behind an empty tmpfs of its own, as Flatpak hides +## `~/.var/app`. Such a tmpfs is recognised by being mounted whole (root `/`) +## below `boundary`, the directory the caller knows to be the host's: the home +## directory for a path inside it, else the granted directory. A tmpfs at or +## above the boundary is the host's own (`/home` on a tmpfs) and blocks +## nothing. Neither does an empty mount list, so unreadable evidence leaves +## the grant's answer standing. +static func flatpak_blocking_mount( + mounts: Array[Dictionary], path: String, boundary: String +) -> Dictionary: + var mount := mount_for_path(mounts, path) + if mount.is_empty(): + return {} + var mount_point := str(mount["mount_point"]) + var host_directory := boundary.simplify_path() + if mount["fstype"] == "tmpfs" and ( + mount_point == "/" + or ( + mount["root"] == "/" + and mount_point != host_directory + and path_is_within(host_directory, mount_point) + ) + ): + return mount + return mount if mount["read_only"] else {} + + +## `path` as the kernel names it, with every symlink among its existing +## components replaced by its target. Mount points are listed by real path, so +## a home spelled `/home/` on an ostree system has to be read as +## `/var/home/` before it can be looked up among them. Components that do +## not exist are kept as written. A chain longer than `_MAX_LINK_HOPS` returns +## the path unchanged. +static func real_path(path: String) -> String: + if OS.get_name() == "Windows": + return path + var remaining := path.simplify_path().split("/", false) + var current := "/" + var hops := 0 + while not remaining.is_empty(): + var parent := DirAccess.open(current) + if parent == null: + break + var candidate := current.path_join(remaining[0]) + remaining.remove_at(0) + if not parent.is_link(candidate): + current = candidate + continue + var target := parent.read_link(candidate) + if hops >= _MAX_LINK_HOPS or target.is_empty(): + return path + hops += 1 + if not target.is_absolute_path(): + target = current.path_join(target) + var target_parts := target.simplify_path().split("/", false) + target_parts.append_array(remaining) + remaining = target_parts + current = "/" + return current if remaining.is_empty() else current.path_join("/".join(remaining)) + + +## Undo the octal escapes mountinfo uses for space, tab, newline and backslash. +static func _unescape_mount_path(field: String) -> String: + if not field.contains("\\"): + return field + var path := "" + var index := 0 + while index < field.length(): + var code := field.substr(index + 1, 3) + if field[index] == "\\" and _is_octal_byte(code): + path += char(int(code[0]) * 64 + int(code[1]) * 8 + int(code[2])) + index += 4 + else: + path += field[index] + index += 1 + return path + + +static func _is_octal_byte(code: String) -> bool: + if code.length() != 3: + return false + for digit in code: + if digit not in "01234567": + return false + return true + + +static func _flatpak_info() -> String: + if not FileAccess.file_exists(_FLATPAK_INFO_PATH): + return "" + var file := FileAccess.open(_FLATPAK_INFO_PATH, FileAccess.READ) + return "" if file == null else file.get_as_text()