Skip to content

Commit 27f930d

Browse files
dsarnoclaude
andauthored
Disconnect debugger screenshot timer on _clear_pending (#297 blind spot) (#339)
* Pin UV_LINK_MODE=copy in uvx-bridge entries to dodge Windows pywin32 lock (#302) * Fall back $HOME -> %USERPROFILE% in path-template expand() so OpenCode auto-configure works on Windows (#318) User on Windows hit "Cannot write to /.config/opencode/opencode.json" and "ERROR: Could not create directory: '/.config'" when auto-configuring OpenCode. The leading "/" came from $HOME substituting to the empty string -- Windows typically only sets USERPROFILE, not HOME. The same module's _home() helper already does this fallback for ~/ expansion; bring $HOME into line so the two spellings mean the same thing on every platform. Running as admin doesn't help because the path itself is malformed (rooted at the drive root). OpenCode is the only descriptor that uses $HOME on Windows (because opencode debug paths reports ~/.config/opencode/opencode.json on every platform); other clients use $APPDATA / $USERPROFILE and dodged the trap. Existing test_opencode_client_uses_home_config_on_windows already exercises the right shape -- passes on Mac/Linux and on GitHub Actions Windows runners (HOME set by default), which is how this slipped through. After this change it passes on a stock Windows box too. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Dedup $HOME fallback to _home() + add mocked-env test for USERPROFILE path (#319) * Dedup $HOME fallback to _home() and lock USERPROFILE coverage with a mocked-env test Follow-up to #318. Two review points from Copilot: 1. The new $HOME -> USERPROFILE branch was a fresh fallback path instead of reusing _home(), so home-directory resolution had two spellings that could drift again. Fold $HOME's fallback through _home() so $HOME and ~ are guaranteed to mean the same thing. 2. test_opencode_client_uses_home_config_on_windows did not actually exercise the new branch on CI: GitHub Actions Windows runners set HOME by default, so the regression check passed without ever touching the USERPROFILE fallback. Add a focused test that explicitly clears HOME and sets USERPROFILE to a known value, runs both $HOME/foo and ~/foo through expand(), and asserts the fallback fires for both spellings on every CI platform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore HOME/USERPROFILE via unset-when-empty pattern to avoid leaking defined-empty env vars Copilot pointed out that OS.set_environment(name, "") creates a defined-empty env var rather than leaving it unset, so restoring via the captured saved_* values would silently promote previously-unset vars to defined-empty for the rest of the test run. Mirror the unset-when-saved-was-empty pattern already used by the GODOT_AI_MODE tests in this file so the original unset/set state is preserved exactly. Also switch the test's HOME-clearing setup from set_environment("HOME", "") to unset_environment("HOME") for the same reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Detect bare-key TOML sections in _toml_strategy to fix codex duplicate-key error (#320) * Detect bare-key TOML sections in clients/_toml_strategy so codex reconfigure doesn't append a duplicate When a user's TOML file has a bare-key section like [mcp_servers.godot-ai] (valid TOML — bare keys allow [A-Za-z0-9_-]+), McpTomlStrategy._all_headers only considered the quoted form [mcp_servers."godot-ai"] that _primary_header always emits. _find_section returned empty, configure fell through to the append-at-end path, and the resulting file had two godot-ai sections — codex's TOML parser rejects this with `duplicate key`. The same bug made check_status report NOT_CONFIGURED on bare-key files (so the dock looked like the client was unconfigured) and made remove a silent no-op. Add _bare_key_header / _is_bare_key helpers and include the bare form in _all_headers when every segment of toml_section_path matches [A-Za-z0-9_-]+. _primary_header keeps emitting the quoted form for new writes (no churn on existing-quoted-form files); the matcher just tolerates either spelling now. Add a regression test covering the codex shape: a fixture file with a bare-key parent section plus a nested .tools subtable that the user might add to set per-tool approval_mode. The test asserts check_status detects the entry, configure updates it in place (no duplicate; subtable customisation survives), and remove cleans both spellings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Extend remove() to clean subtables in the namespace, not just the parent header Copilot pointed out that the original fix in this PR is only partial: remove() matches the parent header [mcp_servers.godot-ai] but stops at the next bracketed line, which on the codex shape is the user's [mcp_servers.godot-ai.tools.session_list] subtable. So remove reports success but leaves the subtable behind. TOML treats that subtable as implicitly defining mcp_servers.godot-ai, so a later configure rewriting [mcp_servers."godot-ai"] produces a duplicate-key error again — the exact shape the original bug took. Add _subtable_prefixes / _matches_subtable_prefix helpers (mirrors of the existing _matches_any_header style) and let remove() consume both the parent header and any subtable header in the namespace before moving on. Configure is unchanged — it only owns the matched parent section's body, leaving subtables alone so user customisations like per-tool approval_mode survive across reconfigure. Tighten the regression test: assert subtable header AND its body (approval_mode line) are gone after remove, then round-trip a configure-after-remove and assert exactly one godot-ai section in the final file. Pre-fix this round-trip would surface the original duplicate-key shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Disconnect debugger screenshot timer on _clear_pending (#297 blind spot) `_clear_pending` previously only erased the dict entry on screenshot success/error, leaving the SceneTreeTimer + bound `_on_timeout` lambda (which captures `request_id`) alive until the timer naturally fired — up to `timeout_sec` (8s default) of dead state per request. Store the bound timeout callable alongside the timer so the cleanup path can disconnect `timer.timeout` and let the lambda + captured request_id drop immediately. `_on_timeout` continues to bypass `_clear_pending` because by the time it runs the timer has already fired and self-disconnected. Closes the "debugger timer leak on screenshot success" blind spot called out in #297. https://claude.ai/code/session_01EaT7NibPToHgffViz4eEvp * Apply Copilot review feedback on TOML section-boundary detection The pre-fix section-end check in remove() and _find_section() required the next header to satisfy `ends_with("]")`, which doesn't hold for the TOML `[next_section] # comment` form (valid TOML, hand-written by some users). The end-of-section loop would walk past such a header and let remove() clobber unrelated sections that came after. Factor a shared `_is_any_section_header(line)` helper that finds the closing `]` and permits whitespace or `#` after it — same shape used by the existing `_matches_any_header` / `_matches_subtable_prefix` helpers elsewhere in the file. remove() and _find_section() both consume it. Adds `test_toml_strategy_remove_tolerates_inline_comment_on_next_header` locking in the regression: file with `[other_section] # comment` after the godot-ai section, remove() must delete only the godot-ai section and preserve [other_section] and its body. script/ci-check-gdscript: clean. pytest: 722 passed. https://claude.ai/code/session_01DwmN9ouQq88KnabiW42y2C * Retrigger CI Game capture smoke / Windows failed in 13s on the previous run — the runtime points at a chickensoft-games/setup-godot@v2 infrastructure hiccup (every other Windows job on the same SHA passed, and the diff in this PR doesn't touch the capture path). Empty commit to retest. https://claude.ai/code/session_01DwmN9ouQq88KnabiW42y2C --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 88e2ebe commit 27f930d

11 files changed

Lines changed: 596 additions & 35 deletions

File tree

.github/workflows/ci.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ jobs:
163163
164164
- name: Start Godot editor (headless)
165165
shell: bash
166-
run: GODOT_AI_ALLOW_HEADLESS=1 godot --headless --path test_project --editor &
166+
run: GODOT_AI_ALLOW_HEADLESS=1 godot --headless --path test_project --editor > godot-editor.log 2>&1 &
167167

168168
- name: Run handler tests
169169
shell: bash
@@ -180,6 +180,15 @@ jobs:
180180
timeout-minutes: 2
181181
run: bash script/ci-quit-test
182182

183+
- name: Dump Godot editor log on failure
184+
if: failure()
185+
shell: bash
186+
run: |
187+
echo "===== godot-editor.log (last 500 lines) ====="
188+
tail -n 500 godot-editor.log || echo "(log not found)"
189+
echo "===== godot-import.log (last 200 lines) ====="
190+
tail -n 200 godot-import.log || echo "(log not found)"
191+
183192
# Separate job from godot-tests: the capture smoke test needs a real
184193
# rendering driver (null RenderingDevice in --headless produces empty
185194
# framebuffers) and a display server. Linux uses xvfb-run; macOS and

README.md

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -173,31 +173,40 @@ every process unmaps it. uv's post-install cleanup of the build venv then
173173
dies on a stale lock; the misleading `pywin32` mention is just the last
174174
package in the resolution order, not the actual lock holder.
175175

176-
**Mitigation in this plugin:** `_stop_server` and `force_restart_server`
177-
both call `McpUvCacheCleanup.purge_stale_builds()` immediately after
178-
killing the server children, while the `.pyd` is briefly unmapped. See
179-
[`plugin/addons/godot_ai/utils/uv_cache_cleanup.gd`](plugin/addons/godot_ai/utils/uv_cache_cleanup.gd).
180-
181-
**Recommended belt-and-braces:** tell uv to copy instead of hard-link by
182-
setting `UV_LINK_MODE=copy` in the MCP launcher's environment. This also
183-
removes the reverse race where an MCP client spawns `uvx mcp-proxy`
184-
*while* a server child is still holding the `.pyd`. Example for Claude
185-
Desktop's `claude_desktop_config.json`:
176+
**Mitigation in this plugin:**
177+
178+
1. `_stop_server` and `force_restart_server` both call
179+
`McpUvCacheCleanup.purge_stale_builds()` immediately after killing the
180+
server children, while the `.pyd` is briefly unmapped. See
181+
[`plugin/addons/godot_ai/utils/uv_cache_cleanup.gd`](plugin/addons/godot_ai/utils/uv_cache_cleanup.gd).
182+
2. **Auto-configure now writes `UV_LINK_MODE=copy` into the bridged
183+
entry's `env` block** for every uvx-bridge client (Claude Desktop, Zed),
184+
telling uv to copy shared C extensions instead of hard-linking them.
185+
That removes the reverse race where an MCP client spawns `uvx mcp-proxy`
186+
*while* a server child still holds the `.pyd`. Existing entries written
187+
by older plugin versions surface in the dock as **drift (amber banner)**
188+
so a single Configure click rewrites them with the env pin.
189+
190+
The shape `client_configure` writes for Claude Desktop is now:
186191

187192
```json
188193
{
189194
"mcpServers": {
190195
"godot-ai": {
191196
"command": "uvx",
192-
"args": ["mcp-proxy", "--transport", "streamablehttp", "http://127.0.0.1:8000/mcp"],
197+
"args": ["mcp-proxy==0.11.0", "--transport", "streamablehttp", "http://127.0.0.1:8000/mcp"],
193198
"env": { "UV_LINK_MODE": "copy" }
194199
}
195200
}
196201
}
197202
```
198203

199-
If you've already hit the lock, kill stray `python.exe` children whose
200-
command line contains `spawn_main(parent_pid=...)` and delete
204+
If you've already hit the lock on an older config, click **Configure**
205+
on the affected uvx-bridge client (Claude Desktop *or* Zed) in the
206+
godot-ai dock to rewrite the entry with the env pin, then quit and
207+
reopen that client. If the lock persists (rare — pre-existing orphans
208+
the cache sweeper couldn't reach), kill stray `python.exe` children
209+
whose command line contains `spawn_main(parent_pid=...)` and delete
201210
`%LOCALAPPDATA%\uv\cache\builds-v0\.tmp*` manually before retrying.
202211

203212
</details>

plugin/addons/godot_ai/clients/_base.gd

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,17 @@ static func resolve_uvx_path() -> String:
180180
## Callers splice this into the client-specific command shape.
181181
static func mcp_proxy_bridge_args(url: String) -> Array:
182182
return ["mcp-proxy==" + MCP_PROXY_VERSION, "--transport", "streamablehttp", url]
183+
184+
185+
## Environment overrides written alongside every auto-configured uvx-bridge
186+
## entry. `UV_LINK_MODE=copy` tells uv to copy shared C extensions into each
187+
## `builds-v0\.tmpXXXXXX\` build venv instead of hard-linking them from
188+
## `archive-v0\`. On Windows that breaks the lock race documented in
189+
## `utils/uv_cache_cleanup.gd` and the README — the running godot-ai server
190+
## holds `_pydantic_core.pyd` mapped, the build venv's hard-linked copy
191+
## inherits the lock, uv's post-install cleanup fails, and the MCP launcher
192+
## reports "pywin32 wheel invalid / file in use" with no working transport.
193+
## Cost on macOS/Linux is a few extra MB in the uvx cache — well worth it
194+
## to keep one config shape across platforms.
195+
static func bridge_env_for_uvx() -> Dictionary:
196+
return {"UV_LINK_MODE": "copy"}

plugin/addons/godot_ai/clients/_json_strategy.gd

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,15 @@ static func build_entry(client: McpClient, server_url: String, existing: Variant
7979
return {
8080
"command": McpClient.resolve_uvx_path(),
8181
"args": McpClient.mcp_proxy_bridge_args(server_url),
82+
"env": _merge_bridge_env(existing),
8283
}
8384
McpClient.UvxBridge.NESTED:
8485
return {
8586
"command": {
8687
"path": McpClient.resolve_uvx_path(),
8788
"args": McpClient.mcp_proxy_bridge_args(server_url),
8889
},
90+
"env": _merge_bridge_env(existing),
8991
"settings": {},
9092
}
9193
var entry: Dictionary = (existing as Dictionary).duplicate() if existing is Dictionary else {}
@@ -113,17 +115,21 @@ static func verify_entry(client: McpClient, entry: Dictionary, server_url: Strin
113115
if entry.get(client.entry_url_field, "") == server_url:
114116
return true
115117
var cmd = entry.get("command", "")
116-
if not (cmd is String):
118+
if not (cmd is String and _command_is_uvx_like(cmd as String)):
117119
return false
118-
var uvx_like := (cmd as String).get_file() == "uvx" or (cmd as String).get_file() == "uvx.exe"
119-
var args = entry.get("args", [])
120-
return uvx_like and args is Array and args.has(server_url)
120+
if not _bridge_args_are_valid(entry.get("args", []), server_url):
121+
return false
122+
return _bridge_env_matches(entry)
121123
McpClient.UvxBridge.NESTED:
122124
var cmd_obj = entry.get("command", {})
123125
if not (cmd_obj is Dictionary):
124126
return false
125-
var nargs = cmd_obj.get("args", [])
126-
return nargs is Array and nargs.has(server_url)
127+
var npath = cmd_obj.get("path", "")
128+
if not (npath is String and _command_is_uvx_like(npath as String)):
129+
return false
130+
if not _bridge_args_are_valid(cmd_obj.get("args", []), server_url):
131+
return false
132+
return _bridge_env_matches(entry)
127133
if entry.get(client.entry_url_field, "") != server_url:
128134
return false
129135
for k in client.entry_extra_fields:
@@ -132,6 +138,72 @@ static func verify_entry(client: McpClient, entry: Dictionary, server_url: Strin
132138
return true
133139

134140

141+
## Pre-fix entries lack `env.UV_LINK_MODE=copy` and hit the Windows uvx
142+
## hard-link race documented in `utils/uv_cache_cleanup.gd`. Flag them as
143+
## drift so the dock surfaces an amber banner and a Configure-click
144+
## rewrites the entry with the env pin. Every key in `bridge_env_for_uvx()`
145+
## must match verbatim — extra user keys are tolerated so a hand-added
146+
## `PYTHONUNBUFFERED=1` etc. doesn't trigger drift forever.
147+
static func _bridge_env_matches(entry: Dictionary) -> bool:
148+
var env = entry.get("env", null)
149+
if not (env is Dictionary):
150+
return false
151+
var pin := McpClient.bridge_env_for_uvx()
152+
for k in pin:
153+
if env.get(k) != pin[k]:
154+
return false
155+
return true
156+
157+
158+
## Configure rewrites the bridge entry wholesale (the bridge form is
159+
## identity-defined by command+args+env), but the verifier tolerates extra
160+
## user-added env keys like `HTTP_PROXY` / `PYTHONUNBUFFERED`. Without
161+
## merging, a Configure click on a CONFIGURED_MISMATCH entry would silently
162+
## drop those keys — so layer the UV_LINK_MODE pin over whatever env block
163+
## already exists on disk. New entries with no prior env get just the pin.
164+
static func _merge_bridge_env(existing: Variant) -> Dictionary:
165+
var pin := McpClient.bridge_env_for_uvx()
166+
if not (existing is Dictionary):
167+
return pin
168+
var existing_env = (existing as Dictionary).get("env", null)
169+
if not (existing_env is Dictionary):
170+
return pin
171+
var merged: Dictionary = (existing_env as Dictionary).duplicate()
172+
for k in pin:
173+
merged[k] = pin[k]
174+
return merged
175+
176+
177+
## Basename match for `uvx` / `uvx.exe`, accepting both the bare-name
178+
## fallback and an absolute path resolved by `McpCliFinder`. Shared by the
179+
## FLAT and NESTED bridge verifiers — the only place we ever inspect a
180+
## stored bridge command/path.
181+
static func _command_is_uvx_like(cmd: String) -> bool:
182+
var basename := cmd.get_file()
183+
return basename == "uvx" or basename == "uvx.exe"
184+
185+
186+
## Strict bridge-argv check: the args array must include the pinned
187+
## `mcp-proxy` package spec, the `--transport streamablehttp` selector, and
188+
## the expected URL. Pre-fix `args.has(url)` was lenient — entries with the
189+
## wrong transport (`--transport sse`) or a different package would still
190+
## verify CONFIGURED, hiding the broken bridge. Match `mcp-proxy` by prefix
191+
## so a future MCP_PROXY_VERSION bump doesn't churn the verifier.
192+
static func _bridge_args_are_valid(args: Variant, server_url: String) -> bool:
193+
if not (args is Array):
194+
return false
195+
var has_mcp_proxy := false
196+
for a in args:
197+
if a is String and (a as String).begins_with("mcp-proxy"):
198+
has_mcp_proxy = true
199+
break
200+
if not has_mcp_proxy:
201+
return false
202+
if not (args.has("--transport") and args.has("streamablehttp") and args.has(server_url)):
203+
return false
204+
return true
205+
206+
135207
## Returns {"ok": true, "data": Dictionary} when the file is absent or parses
136208
## cleanly, and {"ok": false, "error": String} when the file exists with
137209
## non-empty content we cannot safely round-trip. Callers must NOT fall back

plugin/addons/godot_ai/clients/_path_template.gd

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ static func expand(template: String) -> String:
3939
value = _home().path_join("AppData/Roaming")
4040
if value.is_empty() and var_name == "LOCALAPPDATA":
4141
value = _home().path_join("AppData/Local")
42+
if value.is_empty() and var_name == "HOME":
43+
value = _home()
4244
out = out.replace(token, value)
4345
return out
4446

plugin/addons/godot_ai/clients/_toml_strategy.gd

Lines changed: 90 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,20 @@ static func remove(client: McpClient, _server_name: String) -> Dictionary:
8181
return {"status": "error", "message": "Refusing to rewrite %s: %s." % [path, read["error"]]}
8282
var lines: Array[String] = _split_lines(String(read["data"]))
8383
var headers := _all_headers(client)
84+
## Subtables in the namespace (e.g. [mcp_servers.godot-ai.tools.session_list]
85+
## that codex users add to set per-tool approval_mode) must be removed
86+
## too. Leaving them behind keeps `mcp_servers.godot-ai` implicitly
87+
## defined, so a later configure that writes [mcp_servers."godot-ai"]
88+
## produces a duplicate-key TOML error.
89+
var subtable_prefixes := _subtable_prefixes(headers)
8490

8591
var output: Array[String] = []
8692
var i := 0
8793
while i < lines.size():
88-
if _matches_any_header(lines[i], headers):
94+
if _matches_any_header(lines[i], headers) or _matches_subtable_prefix(lines[i], subtable_prefixes):
8995
i += 1
9096
while i < lines.size():
91-
var nt := lines[i].strip_edges()
92-
if nt.begins_with("[") and nt.ends_with("]"):
97+
if _is_any_section_header(lines[i]):
9398
break
9499
i += 1
95100
continue
@@ -152,12 +157,75 @@ static func _primary_header(client: McpClient) -> String:
152157

153158

154159
static func _all_headers(client: McpClient) -> Array[String]:
155-
var out: Array[String] = [_primary_header(client)]
160+
var primary := _primary_header(client)
161+
var out: Array[String] = [primary]
162+
## TOML accepts bare keys ([A-Za-z0-9_-]+) unquoted in section headers,
163+
## so [mcp_servers.godot-ai] is a valid hand-written form of the same
164+
## logical key we emit as [mcp_servers."godot-ai"]. Match both during
165+
## reconfigure / status / remove or a hand-edited (or older-plugin)
166+
## bare-key file gets a duplicate quoted section appended that breaks
167+
## the user's TOML parser.
168+
var bare := _bare_key_header(client)
169+
if not bare.is_empty() and bare != primary:
170+
out.append(bare)
156171
for legacy in client.toml_legacy_section_aliases:
157172
out.append("[%s]" % legacy)
158173
return out
159174

160175

176+
static func _bare_key_header(client: McpClient) -> String:
177+
var parts := client.toml_section_path
178+
if parts.is_empty():
179+
return ""
180+
for p in parts:
181+
if not _is_bare_key(String(p)):
182+
return ""
183+
return "[%s]" % ".".join(parts)
184+
185+
186+
static func _is_bare_key(s: String) -> bool:
187+
if s.is_empty():
188+
return false
189+
for i in range(s.length()):
190+
var c := s.unicode_at(i)
191+
var alpha := (c >= 65 and c <= 90) or (c >= 97 and c <= 122)
192+
var digit := c >= 48 and c <= 57
193+
var dash_or_under := c == 45 or c == 95 # '-' or '_'
194+
if not (alpha or digit or dash_or_under):
195+
return false
196+
return true
197+
198+
199+
## Subtable prefixes derived from each header in `headers`. Strips the
200+
## closing `]` and appends `.` so a header `[a.b]` becomes the prefix
201+
## `[a.b.` — matching subtables `[a.b.<rest>]` but NOT siblings like
202+
## `[a.b-other]` (next char must be a dot, not anything bare-key-valid).
203+
static func _subtable_prefixes(headers: Array[String]) -> Array[String]:
204+
var out: Array[String] = []
205+
for h in headers:
206+
if h.length() > 2 and h.ends_with("]"):
207+
out.append(h.substr(0, h.length() - 1) + ".")
208+
return out
209+
210+
211+
## Mirror of `_matches_any_header` for subtable prefixes — line must
212+
## start with `[a.b.` and have a closing `]` followed only by whitespace
213+
## or a comment.
214+
static func _matches_subtable_prefix(line: String, prefixes: Array[String]) -> bool:
215+
var trimmed := line.strip_edges()
216+
for p in prefixes:
217+
if not trimmed.begins_with(p):
218+
continue
219+
var rest := trimmed.substr(p.length())
220+
var bracket := rest.find("]")
221+
if bracket < 0:
222+
continue
223+
var remainder := rest.substr(bracket + 1).strip_edges()
224+
if remainder.is_empty() or remainder.begins_with("#"):
225+
return true
226+
return false
227+
228+
161229
## Exact-header match. We cannot use a simple prefix check because
162230
## `[mcp_servers."godot-ai"` is a prefix of `[mcp_servers."godot-ai-dev"]`,
163231
## which would silently delete unrelated sections during remove().
@@ -177,9 +245,25 @@ static func _find_section(lines: Array[String], headers: Array[String]) -> Dicti
177245
if _matches_any_header(lines[i], headers):
178246
var end := lines.size()
179247
for j in range(i + 1, lines.size()):
180-
var nt := lines[j].strip_edges()
181-
if nt.begins_with("[") and nt.ends_with("]"):
248+
if _is_any_section_header(lines[j]):
182249
end = j
183250
break
184251
return {"start": i, "end": end}
185252
return {}
253+
254+
255+
## Generic "is this line a TOML section header" check that tolerates an
256+
## inline comment after the closing `]`, e.g. `[next_section] # note`.
257+
## The pre-fix `nt.begins_with("[") and nt.ends_with("]")` rejected those
258+
## lines, so a hand-written comment after a header would let the
259+
## section-deletion / section-end loops walk straight through into the
260+
## following section and clobber unrelated content.
261+
static func _is_any_section_header(line: String) -> bool:
262+
var trimmed := line.strip_edges()
263+
if not trimmed.begins_with("["):
264+
return false
265+
var bracket := trimmed.find("]")
266+
if bracket < 0:
267+
return false
268+
var remainder := trimmed.substr(bracket + 1).strip_edges()
269+
return remainder.is_empty() or remainder.begins_with("#")

plugin/addons/godot_ai/debugger/mcp_debugger_plugin.gd

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ const GAME_READY_WAIT_SEC := 20.0
3232
var _log_buffer: McpLogBuffer
3333
var _game_log_buffer: McpGameLogBuffer
3434

35-
## Pending request_id -> {connection, deadline_ts, timer}
35+
## Pending request_id -> {connection, timer, timeout_callable}.
36+
## We retain the bound timeout lambda so `_clear_pending` can disconnect
37+
## it on success/error; otherwise the SceneTreeTimer pins the captured
38+
## request_id until `timeout_sec` elapses (8s default).
3639
var _pending: Dictionary = {}
3740

3841
## Flipped true when the game-side autoload sends its "mcp:hello" boot
@@ -216,10 +219,14 @@ func _send_take_screenshot(
216219
"No active debugger session — is the game actually running and started from this editor?")
217220
return
218221

219-
_pending[request_id] = {"connection": connection}
220222
var timer: SceneTreeTimer = tree.create_timer(timeout_sec)
221-
timer.timeout.connect(func() -> void: _on_timeout(request_id))
222-
_pending[request_id]["timer"] = timer
223+
var timeout_callable := func() -> void: _on_timeout(request_id)
224+
timer.timeout.connect(timeout_callable)
225+
_pending[request_id] = {
226+
"connection": connection,
227+
"timer": timer,
228+
"timeout_callable": timeout_callable,
229+
}
223230

224231
session.send_message("mcp:take_screenshot", [request_id, max_resolution])
225232
if _log_buffer:
@@ -300,4 +307,9 @@ func _send_error(connection: McpConnection, request_id: String, code: String, me
300307

301308

302309
func _clear_pending(request_id: String) -> void:
310+
var pending: Dictionary = _pending.get(request_id, {})
311+
var timer: SceneTreeTimer = pending.get("timer")
312+
var cb: Callable = pending.get("timeout_callable", Callable())
313+
if timer != null and timer.timeout.is_connected(cb):
314+
timer.timeout.disconnect(cb)
303315
_pending.erase(request_id)

0 commit comments

Comments
 (0)