feat(connection): honour GODOT_MCP_SERVER_PATH via the env/.env layer

[IvanMurzak]

Summary

• Registers GODOT_MCP_SERVER_PATH in the addon's existing env resolution layer — process env > project res://.env (GodotMcpEnvFile.LookupRaw) > none, the same precedence GODOT_MCP_DEV_CONTROL uses — so a chain/CI-built gamedev-mcp-server can be launched instead of the release pinned by GodotMcpServerView.ServerVersion. Never a bare Environment.GetEnvironmentVariable.
• While the override is active the editor skips the release download (before the CI check, so a CI/chain run gets its own binary rather than the "skipped in CI" refusal), treats IsVersionMatches() as true (an arbitrary build carries no version marker), launches with the override binary's own directory as the working directory, and skips orphaned-server cleanup. The last one is a correctness requirement: GodotMcpServerOwnership.IsOwnedByThisProject matches on the same containing directory, and an override binary is shared by design, so the cleanup would kill a sibling project's or harness's live server.
• A value that names no existing file is ignored and the normal download path runs — Unreal's UNREAL_MCP_SERVER_PATH / ResolveBinaryPath rule.
• All decisions live in a new pure-managed resolver, GodotMcpServerPathOverride (the DevControlGate pattern): the caller does the env/file I/O and passes raw strings plus a fileExists delegate. This is deliberate — the manager statics that would otherwise carry them (ExecutableFullPath / IsVersionMatches / KillOrphanedServerProcesses) are #if TOOLS and reach ProjectSettings.GlobalizePath, which faults in the plain-xUnit host, so they are not unit-testable at all; factoring the decisions out is what makes them pinnable.

Taskflow 2026-09-03-chain-testing, row p1-server-path-godot (goal G13) — the one permitted product code change for this plugin (design decision D3 / C30).

Files

File	Why
addons/godot_mcp/Runtime/GodotMcpEnv.cs	+ ServerPath = "GODOT_MCP_SERVER_PATH" in the canonical env-name class.
addons/godot_mcp/Editor/Connection/GodotMcpServerPathOverride.cs (new)	Pure resolver: normalization, precedence, existence gate, launch path, working directory.
addons/godot_mcp/Editor/Connection/GodotMcpServerManager.cs	Resolve-once-per-boot wiring + the four call sites; new CachedExecutableFullPath() keeps the download target override-free.
Godot-MCP.Tests/GodotMcpServerPathOverrideTests.cs (new)	19 tests / 24 cases.
Godot-MCP.Tests/Godot-MCP.Tests.csproj	<Compile Include> for the new resolver source.
README.md	One env-table row + an editor-only clarifying note.

.github/** diff is empty (owned by p2-dispatch-godot-mcp). Godot-MCP.csproj pins, Godot-Tests/**, cli/**, plugin.cfg and GodotMcpServerView.ServerVersion are untouched.

Test plan

• Target-specific local tests passed (see profile test.md).
• Suite 1 — dotnet restore + dotnet build Godot-MCP.sln --configuration Debug --no-restore: 0 errors, 0 warnings.
• Suite 2 — dotnet test Godot-MCP.Tests/...: 1334 passed / 0 failed (was 1310 before this change; +24 new test methods — 19 from the first push, 5 more from the refine pass below).
• Plant round, RE-RUN and EXTENDED against the final tree (0a58c26): 22/22 matched their expect | 22 RED | 0 GREEN | restore-failures 0, harness exit 0, source restored byte-exactly (md5 c164fbc6451f477401312079fbd715fe before and after). Every one of the 22 per-plant logs collected Total tests: 1334 — identical to the clean run — so no RED came from an import/compile break.
• Behavioural proof in a real headless Godot 4.5.1 mono editor, both directions (below).
• Suite 2b (cli/ npm) — not applicable: no cli/ file in the diff and no tool-surface / ToolType change, which are the only triggers test.md lists for it.

Plant table — every RED attributed from its own per-plant log

Each plant reverts exactly one claim; the verify command rebuilds and re-runs the suite
(dotnet build … && dotnet test … --no-build --verbosity normal), so a plant cannot score against a
stale assembly. All 22 per-plant logs collected Total tests: 1334 — identical to the clean-baseline run on
this same tree — so no RED came from an import/compile break. The round was re-run against the
final tree 0a58c26 with every per-plant log retained; SUMMARY 22/22 matched their expect | 22 RED | 0 GREEN | restore-failures 0, harness exit 0, and digest_before == digest_after on all 22 restores.
P20/P21 share one marker and are discriminated by assertion, not by test name (Assert.False at
line 284 vs Assert.True at line 280).

#	Plant (mutation)	Expect	Attributed RED (from NN-<name>.log)	Counts
P1	drop the fileExists gate (return normalized;)	red	Resolve_SetButMissingFile_ReturnsNull (+6 siblings incl. Resolve_ExistingFile_ReturnsThatPath, whose recorded-probe assertion is what sees a removed gate)	1327/7
P2	invert the fileExists gate	red	Resolve_ExistingFile_ReturnsThatPath (+6)	1327/7
P3	drop the double-quote/whitespace normalizer	red	Resolve_DoubleQuotedValue_TrimsQuotesBeforeTheExistenceGate, Normalize_TrimsWhitespaceThenOnePairOfQuotes, SelectRaw_AppliesPrecedenceAndNormalization	1330/4
P4	drop the single-quote strip	red	Resolve_SingleQuotedValue_TrimsQuotesBeforeTheExistenceGate (+2)	1331/3
P5	reverse precedence (.env beats process env)	red	Resolve_BothLayersSet_ProcessValueWins, SelectRaw_AppliesPrecedenceAndNormalization	1332/2
P6	ExecutablePath ignores the override	red	ExecutablePath_OverrideActive_LaunchesTheOverride	1333/1
P7	ExecutablePath always uses the override	red	ExecutablePath_NoOverride_LaunchesTheCachedBinary	1333/1
P8	WorkingDirectory always returns the fallback	red	WorkingDirectory_IsTheDirectoryOfTheResolvedExecutable	1332/2
P9	WorkingDirectory drops the fallback	red	WorkingDirectory_PathWithoutADirectoryComponent_FallsBack	1333/1
P10	IsActive always true	red	IsActive_OnlyForANonEmptyResolvedOverride, ExecutablePath_NoOverride_LaunchesTheCachedBinary	1329/5
Both directions are covered for every gate: P1/P2 (existence gate), P3/P4 (the two quote conventions),
P5 (precedence, with both candidate paths existing so the existence gate cannot be what picks the
winner), P6/P7 (override vs cached launch path), P8/P9 (working directory), P10 (the predicate).
Several tests assert the recorded fileExists argument rather than only a null/non-null
result — an absence assertion alone cannot distinguish "the gate refused the value" from "the value
never reached the gate", and P1 is precisely the mutation that exploits that difference.

Added by the refine pass (P11-P22). The verify command is unchanged, so these ran under the same
rebuild-then-retest shape as P1-P10.

#	Plant (mutation)	Expect	Attributed RED (from NN-<name>.log)	Counts
P11	blank value reaches the existence gate (normalized = string.Empty instead of returning)	red	Resolve_NullEmptyOrWhitespace_ReturnsNullWithoutProbing	1329/5
P12	Normalize stops collapsing blank to null (return trimmed;)	red	Normalize_NullEmptyOrWhitespace_ReturnsNull	1324/10
P13	drop Resolve's fileExists null guard	red	Resolve_NullFileExistsDelegate_Throws	1333/1
P14	3-arg Resolve drops the .env layer	red	Resolve_ProcessValueBlank_FallsBackToEnvFileValue	1333/1
P15	swap the single-quote strip and the shared double-quote normalizer	red	Normalize_StripsSingleQuotesBEFORETheSharedDoubleQuoteNormalizer	1333/1
P16	ShouldKillOrphans polarity inverted	red	ShouldKillOrphans_OnlyWithoutAnOverride	1333/1
P17	VersionMatchesOrOverridden || → && (override stops forcing true)	red	VersionMatchesOrOverridden_TrueUnderOverride… (Assert.True, line 247)	1332/2
P18	same method loses ONLY its short-circuit (result unchanged)	red	VersionMatchesOrOverridden_TrueUnderOverride… (Assert.False(consulted), line 251)	1333/1
P19	VersionMatchesOrOverridden stops deferring without an override (return true;)	red	VersionMatchesOrOverridden_NoOverride_DefersToTheCachedVersionVerdict	1333/1
P20	IsIgnoredValue fires when nothing was supplied (drop the raw check)	red	IsIgnoredValue_TrueOnlyWhen… (Assert.False, line 284)	1333/1
P21	IsIgnoredValue never fires (=> false)	red	IsIgnoredValue_TrueOnlyWhen… (Assert.True, line 280)	1333/1
P22	WorkingDirectory no-override answer comes from the fallback (ternary inverted)	red	WorkingDirectory_NoOverride_IsStillTheCacheFolder	1331/3

Two plants share a marker in each of two groups (P17/P18 and P20/P21), and both groups were
hand-checked to fail DIFFERENTLY — the harness's own shared-marker comparator extracts a pytest
FAILURES block and so reports could NOT be compared on this xUnit profile, which is not a pass.
Read from the per-plant logs: P17 fails Assert.True at line 247 while P18 fails Assert.False(consulted)
at line 251; P21 fails Assert.True at line 280 while P20 fails Assert.False at line 284. Distinct
assertion, line and text in both groups.

Refine pass (0a58c26)

Four report-only review helpers plus an in-context pass. Behaviour-preserving except for one added
diagnostic. What it changed and why:

• A set-but-unresolvable override was ignored in complete silence — byte-indistinguishable from
  "not set", so a CI or dev run whose purpose is to exercise its OWN server build would pass against
  the downloaded release with nothing in the log. New pure IsIgnoredValue + a warning at the boot
  site, which now caches the selected-but-ungated raw value alongside the gated one (P20/P21).
• DownloadAndUnpackBinary's post-unpack verdict could not fail under an override: it used the
  launch-scoped IsBinaryExists()/IsVersionMatches() pair, both of which an active override answers
  unconditionally. Re-scoped to the cache, matching the CachedExecutableFullPath() choice the same
  hunk already made for the unpack target. Unreachable under override today, so it is a no-op on every
  currently reachable path — the fix removes a latent trap rather than a live bug.
• DoD 3 is now literally satisfied. The two under-override decisions were a bare || and an early
  return inside GodotMcpServerManager, which is #if TOOLS and is not compiled into the xUnit host,
  so their POLARITY was pinned by nothing — only the IsActive input was. Factored into
  VersionMatchesOrOverridden (delegate-valued, so the cache read keeps its short-circuit) and
  ShouldKillOrphans (P16-P19).
• A test that could not fail: WorkingDirectory_NoOverride_IsStillTheCacheFolder passed a fallback
  equal to the executable's own directory, so both arms of the method returned the same string and
  neither P8 nor P9 could redden it. It now passes a fallback the answer must not be (P22).
• Two shared-marker pairs made discriminable by reordering assertions rather than weakening any.
• Docs: IsBinaryExists()'s summary still said "the cached executable" after ExecutableFullPath()
  became override-aware underneath it; the README's "read once per editor session" was false (the cache
  is per assembly load — a C# hot-reload re-reads it, a plugin toggle does not); added the
  ignored-value warning and the Unix executable-bit caveat.

Recorded, deliberately NOT changed (each was raised by a helper and rejected on evidence):
GodotMcpServerPathOverride.Normalize duplicates the private GodotMcpEnvFile.Sanitize — sharing it
would edit a file outside the task's capped product-diff set. Removing the double normalization on the
production path is a BEHAVIOUR change, not a cleanup: "'…'" unwraps today and would stop, so it is
documented instead. A relative override path is accepted by the existence gate but then does not get
its own directory as the working directory — documented on WorkingDirectory; rejecting non-rooted
values was declined as an unforced behaviour restriction. Two helpers recommended deleting
VersionMatchesOrOverridden / ShouldKillOrphans as a || b and !a re-implemented — declined
because DoD 3 requires exactly that factoring, with the class doc updated so the file no longer reads
as contradicting itself.

Honest limitation. The download-skip, version-match bypass and orphan-cleanup skip are ONE
decision (IsActive) read at three call sites, not three independent flags — so they are entailed by
each other and one plant (P10) covers the predicate. The manager call sites themselves carry no
unit plant: GodotMcpServerManager is #if TOOLS and is not compiled into the xUnit assembly, so any
plant there would score a (correct but useless) GREEN. The refine pass above closed the two that WERE
expressible as pure decisions (VersionMatchesOrOverridden, ShouldKillOrphans, plants P16-P19); what remains uncovered by a unit plant is the manager WIRING itself — the early return's position BEFORE the CI check, and the four call sites — covered by the compile and by the behavioural proof below.

Behavioural proof — headless Godot 4.5.1 mono, engines/godot/test-project

Held-open --editor session with the dev-control bridge, POST /control/click {"target":"start-server"}.

With GODOT_MCP_SERVER_PATH set to a locally built gamedev-mcp-server.exe:

[Godot-MCP] using GODOT_MCP_SERVER_PATH override: C:/.../.agent-scratch/chain-server/gamedev-mcp-server.exe (release download and version match skipped).
[Godot-MCP] orphaned-server cleanup skipped: GODOT_MCP_SERVER_PATH override is active (C:/.../chain-server/gamedev-mcp-server.exe); the binary may be shared with other projects or processes.
[Godot-MCP] local server process started (PID 60712, port 22712); verifying…
[Godot-MCP] local server verified and running.

grep -c "downloading GameDev-MCP-Server" = 0, and test-project/.godot/mcp-server/ was ABSENT after the run.

Same testbed, same editor, variable UNSET (control):

[Godot-MCP] downloading GameDev-MCP-Server binary from: https://github.com/IvanMurzak/GameDev-MCP-Server/releases/download/v9.2.5/gamedev-mcp-server-win-x64.zip
[Godot-MCP] local server process started (PID 10884, port 22712); verifying…
[Godot-MCP] local server verified and running.

.godot/mcp-server/win-x64/ was then present with gamedev-mcp-server.exe + the version marker.

Local-verification caveat, for the record: engines/godot/test-project/Godot-Test-Project.csproj
pins com.IvanMurzak.McpPlugin 7.5.2 while this addon pins 8.3.0, so the testbed cannot
compile the addon as checked in — measured: CS0246 on LoginCommitStatus, TokenRefreshRequest,
HttpTokenRefresher, MachineCredentialLock, … in GodotAccountAuth.cs / GodotTokenRefresher.cs,
files this PR does not touch. That is the pre-existing testbed drift the csproj's own comment warns
about. The smoke above was run behind a temporary, local, reverted bump of that testbed pin to
8.3.0; nothing in that repo was committed. The lockstep bump itself belongs to a separate task.

Notes for the reviewer

• The override is resolved once per boot (per assembly load) and cached behind a lock, so the launch path, the version-match bypass and the cleanup skip can never disagree mid-session.
• ExecutableFullPath() is now "what do we launch"; the new CachedExecutableFullPath() is "where does the download land". DownloadAndUnpackBinary uses the latter, so unpacking can never be redirected at the override.
• docs/runtime-security.md's env table was deliberately not touched: it is scoped to the game-build runtime config read by GodotMcpConfig, and this variable is editor-only.

CI on the final head 0a58c264d039b6a893d6fe314cf9b1bb907f226b

Workflow	Run	Conclusion
CI	33809312386	success
CI	33809312931	success
Test Pull Request	33809312573	success

All three runs are tied to headSha 0a58c264d039b6a893d6fe314cf9b1bb907f226b, the current head of this
branch — status-check rollup 28/28 COMPLETED/SUCCESS at that SHA.

Superseded, kept for history: the first push 8966f2d was green on
33804240480 (CI) and
33804240776 (Test Pull Request).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bac1LKpVobv1i1FNGCRNvM