Skip to content

Commit 04e0ffa

Browse files
authored
feat(connection): honour GODOT_MCP_SERVER_PATH via the env/.env layer (#354)
Godot addon honours GODOT_MCP_SERVER_PATH through the process-env > project .env layer: an existing file is launched instead of the pinned release, skipping the download and the version match, with orphaned-server cleanup disabled while the override is active.
1 parent b91650c commit 04e0ffa

6 files changed

Lines changed: 755 additions & 11 deletions

File tree

Godot-MCP.Tests/Godot-MCP.Tests.csproj

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,13 @@
285285
unit-tested here (cross-platform: every method is a string/enum transform, so the same assertions
286286
hold on the Linux CI runner). -->
287287
<Compile Include="..\addons\godot_mcp\Runtime\Connection\GodotMcpServerView.cs" Link="Source\GodotMcpServerView.cs" />
288+
<!-- GODOT_MCP_SERVER_PATH override RESOLVER — pure-managed (no Godot native types, no #if TOOLS, no I/O of
289+
its own: the caller passes raw env/.env strings plus a fileExists delegate). It carries every decision
290+
the override drives — precedence, quote/whitespace normalization, the existing-file gate, the launch
291+
path and the child process's working directory — because the manager statics that would otherwise hold
292+
them (ExecutableFullPath / IsVersionMatches / KillOrphanedServerProcesses, GodotMcpServerManager.cs,
293+
#if TOOLS) all reach ProjectSettings.GlobalizePath and cannot run in this binary-less host. -->
294+
<Compile Include="..\addons\godot_mcp\Editor\Connection\GodotMcpServerPathOverride.cs" Link="Source\GodotMcpServerPathOverride.cs" />
288295
<!-- Segmented-control state model — pure-managed (no Godot native types, no #if TOOLS): the
289296
value→index / selected-predicate / clamp rules for the reusable segmented control (Custom|Cloud,
290297
none|required). The editor builder (DockStyle.SegmentedControl, #if TOOLS) consumes these and is
Lines changed: 364 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
/*
2+
┌──────────────────────────────────────────────────────────────────┐
3+
│ Author: Ivan Murzak (https://github.com/IvanMurzak) │
4+
│ Repository: GitHub (https://github.com/IvanMurzak/Godot-MCP) │
5+
│ Copyright (c) 2026 Ivan Murzak │
6+
│ Licensed under the Apache License, Version 2.0. │
7+
│ See the LICENSE file in the project root for more information. │
8+
└──────────────────────────────────────────────────────────────────┘
9+
*/
10+
#nullable enable
11+
using System;
12+
using System.Collections.Generic;
13+
using System.IO;
14+
using com.IvanMurzak.Godot.MCP.Connection;
15+
using Xunit;
16+
17+
namespace com.IvanMurzak.Godot.MCP.Tests
18+
{
19+
/// <summary>
20+
/// Pins the <c>GODOT_MCP_SERVER_PATH</c> override resolver
21+
/// (<see cref="GodotMcpServerPathOverride"/>) — the dev/CI escape hatch that makes the editor launch a
22+
/// caller-supplied <c>gamedev-mcp-server</c> instead of the pinned release, skipping the download and the
23+
/// cached-version match.
24+
///
25+
/// <para>
26+
/// This class is the ONLY automated gate for those decisions: the manager methods that consume them
27+
/// (<c>GodotMcpServerManager.ExecutableFullPath</c> / <c>IsVersionMatches</c> /
28+
/// <c>KillOrphanedServerProcesses</c>) are <c>#if TOOLS</c> and reach
29+
/// <c>ProjectSettings.GlobalizePath</c>, which faults in this binary-less xUnit host — so the decisions
30+
/// were factored into this pure resolver precisely so they could be pinned here.
31+
/// </para>
32+
///
33+
/// <para>
34+
/// Every existence check goes through <see cref="RecordingFileExists"/>, which also RECORDS the exact
35+
/// string it was asked about. Several tests assert that recorded argument rather than only the
36+
/// <c>null</c>/non-<c>null</c> result: an absence assertion alone cannot tell "the gate rejected the
37+
/// value" from "the value never arrived", and the recorded probe is the positive artifact that does.
38+
/// </para>
39+
/// </summary>
40+
public class GodotMcpServerPathOverrideTests
41+
{
42+
/// <summary>A <c>fileExists</c> double that answers from a fixed set AND records every probe.</summary>
43+
sealed class RecordingFileExists
44+
{
45+
readonly HashSet<string> _existing;
46+
47+
public RecordingFileExists(params string[] existing)
48+
=> _existing = new HashSet<string>(existing, StringComparer.Ordinal);
49+
50+
public List<string> Probes { get; } = new();
51+
52+
public bool Exists(string path)
53+
{
54+
Probes.Add(path);
55+
return _existing.Contains(path);
56+
}
57+
58+
public Func<string, bool> Delegate => Exists;
59+
}
60+
61+
static string ExePath(params string[] segments)
62+
{
63+
var parts = new List<string> { Path.GetTempPath(), "godot-mcp-server-path-override-tests" };
64+
parts.AddRange(segments);
65+
return Path.Combine(parts.ToArray());
66+
}
67+
68+
// --- an existing file resolves to exactly that path ------------------------------------------
69+
70+
[Fact]
71+
public void Resolve_ExistingFile_ReturnsThatPath()
72+
{
73+
var exe = ExePath("chain", "gamedev-mcp-server.exe");
74+
var files = new RecordingFileExists(exe);
75+
76+
Assert.Equal(exe, GodotMcpServerPathOverride.Resolve(exe, files.Delegate));
77+
Assert.Equal(new[] { exe }, files.Probes);
78+
}
79+
80+
// --- set, but naming no existing file, falls THROUGH (Unreal's ResolveBinaryPath rule) --------
81+
82+
[Fact]
83+
public void Resolve_SetButMissingFile_ReturnsNull()
84+
{
85+
var missing = ExePath("chain", "not-published-yet", "gamedev-mcp-server.exe");
86+
var files = new RecordingFileExists(/* nothing exists */);
87+
88+
var resolved = GodotMcpServerPathOverride.Resolve(missing, files.Delegate);
89+
90+
// The probe assertion comes FIRST deliberately. Positive artifact: the value DID reach the
91+
// existence gate with its full path, so the null below is the gate refusing a missing file — not
92+
// the value being dropped before the gate. Ordering it first also keeps the two gate mutations
93+
// tellable apart: REMOVING the gate fails here (no probe was ever made), whereas INVERTING it
94+
// probes normally and fails on the null instead.
95+
Assert.Equal(new[] { missing }, files.Probes);
96+
Assert.Null(resolved);
97+
}
98+
99+
// --- unset / blank values resolve to null and never touch the filesystem ----------------------
100+
101+
[Theory]
102+
[InlineData(null)]
103+
[InlineData("")]
104+
[InlineData(" ")]
105+
[InlineData("\t\r\n")]
106+
public void Resolve_NullEmptyOrWhitespace_ReturnsNullWithoutProbing(string? raw)
107+
{
108+
var files = new RecordingFileExists(ExePath("chain", "gamedev-mcp-server.exe"));
109+
110+
Assert.Null(GodotMcpServerPathOverride.Resolve(raw, files.Delegate));
111+
Assert.Empty(files.Probes);
112+
}
113+
114+
[Fact]
115+
public void Normalize_NullEmptyOrWhitespace_ReturnsNull()
116+
{
117+
Assert.Null(GodotMcpServerPathOverride.Normalize(null));
118+
Assert.Null(GodotMcpServerPathOverride.Normalize(""));
119+
Assert.Null(GodotMcpServerPathOverride.Normalize(" "));
120+
}
121+
122+
// --- surrounding quotes + whitespace are trimmed, the .env layer's convention -----------------
123+
124+
[Fact]
125+
public void Resolve_DoubleQuotedValue_TrimsQuotesBeforeTheExistenceGate()
126+
{
127+
var exe = ExePath("chain", "gamedev-mcp-server.exe");
128+
var files = new RecordingFileExists(exe);
129+
130+
Assert.Equal(exe, GodotMcpServerPathOverride.Resolve("\"" + exe + "\"", files.Delegate));
131+
132+
// The gate must be asked about the UNQUOTED path; asking about the quoted one would silently
133+
// ignore a perfectly good override exported as GODOT_MCP_SERVER_PATH="C:/.../server.exe".
134+
Assert.Equal(new[] { exe }, files.Probes);
135+
}
136+
137+
[Fact]
138+
public void Resolve_SingleQuotedValue_TrimsQuotesBeforeTheExistenceGate()
139+
{
140+
var exe = ExePath("chain", "gamedev-mcp-server");
141+
var files = new RecordingFileExists(exe);
142+
143+
Assert.Equal(exe, GodotMcpServerPathOverride.Resolve("'" + exe + "'", files.Delegate));
144+
Assert.Equal(new[] { exe }, files.Probes);
145+
}
146+
147+
[Fact]
148+
public void Resolve_SurroundingWhitespace_IsTrimmed()
149+
{
150+
var exe = ExePath("chain", "gamedev-mcp-server");
151+
var files = new RecordingFileExists(exe);
152+
153+
// Documents the contract, but do NOT read it as coverage of Normalize's own leading Trim():
154+
// GodotMcpConfig.NormalizeEnv trims too, so this case stays green with the local trim removed.
155+
// The whitespace case that is actually load-bearing is the QUOTED-and-spaced one in
156+
// Normalize_TrimsWhitespaceThenOnePairOfQuotes — without the local trim the leading space means
157+
// the value no longer starts with a quote and the single-quote branch is skipped entirely.
158+
Assert.Equal(exe, GodotMcpServerPathOverride.Resolve(" " + exe + "\t", files.Delegate));
159+
Assert.Equal(new[] { exe }, files.Probes);
160+
}
161+
162+
[Fact]
163+
public void Normalize_TrimsWhitespaceThenOnePairOfQuotes()
164+
{
165+
Assert.Equal("/srv/gamedev-mcp-server", GodotMcpServerPathOverride.Normalize(" \"/srv/gamedev-mcp-server\" "));
166+
Assert.Equal("/srv/gamedev-mcp-server", GodotMcpServerPathOverride.Normalize(" '/srv/gamedev-mcp-server' "));
167+
Assert.Equal("/srv/gamedev-mcp-server", GodotMcpServerPathOverride.Normalize("/srv/gamedev-mcp-server"));
168+
}
169+
170+
[Fact]
171+
public void Normalize_StripsSingleQuotesBEFORETheSharedDoubleQuoteNormalizer()
172+
{
173+
// The ONLY inputs that can tell the two orders apart are NESTED pairs — every single-convention
174+
// value normalizes identically either way, which is why the ordering claim in Normalize's
175+
// docstring needs a case of its own rather than riding on the cases above.
176+
//
177+
// Current order (single strip, THEN GodotMcpConfig.NormalizeEnv's Trim('"')): the outer double
178+
// quotes come off and the inner single quotes survive. The swapped order would strip both and
179+
// return "/srv/x". This matches GodotMcpEnvFile.Sanitize, which is the compatibility contract:
180+
// a value must normalize the same whether it arrived from the process env or from res://.env.
181+
Assert.Equal("'/srv/x'", GodotMcpServerPathOverride.Normalize("\"'/srv/x'\""));
182+
}
183+
184+
// --- precedence: the PROCESS value beats the project .env value -------------------------------
185+
186+
[Fact]
187+
public void Resolve_BothLayersSet_ProcessValueWins()
188+
{
189+
var fromProcess = ExePath("process", "gamedev-mcp-server.exe");
190+
var fromEnvFile = ExePath("dotenv", "gamedev-mcp-server.exe");
191+
// BOTH exist, so the existence gate cannot be what picks the winner — only precedence can.
192+
var files = new RecordingFileExists(fromProcess, fromEnvFile);
193+
194+
Assert.Equal(fromProcess, GodotMcpServerPathOverride.Resolve(fromProcess, fromEnvFile, files.Delegate));
195+
Assert.Equal(new[] { fromProcess }, files.Probes);
196+
}
197+
198+
[Fact]
199+
public void Resolve_ProcessValueBlank_FallsBackToEnvFileValue()
200+
{
201+
var fromEnvFile = ExePath("dotenv", "gamedev-mcp-server.exe");
202+
var files = new RecordingFileExists(fromEnvFile);
203+
204+
Assert.Equal(fromEnvFile, GodotMcpServerPathOverride.Resolve(" ", fromEnvFile, files.Delegate));
205+
Assert.Equal(new[] { fromEnvFile }, files.Probes);
206+
}
207+
208+
[Fact]
209+
public void Resolve_NeitherLayerSet_ReturnsNull()
210+
{
211+
var files = new RecordingFileExists(ExePath("dotenv", "gamedev-mcp-server.exe"));
212+
213+
Assert.Null(GodotMcpServerPathOverride.Resolve(null, "", files.Delegate));
214+
Assert.Empty(files.Probes);
215+
}
216+
217+
[Fact]
218+
public void SelectRaw_AppliesPrecedenceAndNormalization()
219+
{
220+
Assert.Equal("/a/server", GodotMcpServerPathOverride.SelectRaw("\"/a/server\"", "/b/server"));
221+
Assert.Equal("/b/server", GodotMcpServerPathOverride.SelectRaw(" ", " '/b/server' "));
222+
Assert.Null(GodotMcpServerPathOverride.SelectRaw(null, " "));
223+
}
224+
225+
// --- the single "override in force" predicate every consumer reads --------------------------------
226+
227+
[Fact]
228+
public void IsActive_OnlyForANonEmptyResolvedOverride()
229+
{
230+
Assert.True(GodotMcpServerPathOverride.IsActive("/srv/gamedev-mcp-server"));
231+
Assert.False(GodotMcpServerPathOverride.IsActive(null));
232+
Assert.False(GodotMcpServerPathOverride.IsActive(""));
233+
}
234+
235+
// --- the two under-override decisions the manager itself cannot pin --------------------------------
236+
//
237+
// GodotMcpServerManager is #if TOOLS and reaches ProjectSettings.GlobalizePath, so it does not compile
238+
// into this host and its call sites can never be asserted here. These are the decisions themselves,
239+
// factored out so the POLARITY of each is pinned rather than only the predicate they read.
240+
241+
[Fact]
242+
public void VersionMatchesOrOverridden_TrueUnderOverrideWithoutConsultingTheCachedVersion()
243+
{
244+
var consulted = false;
245+
Func<bool> cached = () => { consulted = true; return false; };
246+
247+
Assert.True(GodotMcpServerPathOverride.VersionMatchesOrOverridden("/srv/gamedev-mcp-server", cached));
248+
249+
// Positive artifact for the short-circuit: reading the cache's `version` marker is file I/O and
250+
// that folder routinely does not exist under an override, so it must not be reached at all.
251+
Assert.False(consulted);
252+
}
253+
254+
[Fact]
255+
public void VersionMatchesOrOverridden_NoOverride_DefersToTheCachedVersionVerdict()
256+
{
257+
Assert.True(GodotMcpServerPathOverride.VersionMatchesOrOverridden(null, () => true));
258+
Assert.False(GodotMcpServerPathOverride.VersionMatchesOrOverridden(null, () => false));
259+
Assert.False(GodotMcpServerPathOverride.VersionMatchesOrOverridden("", () => false));
260+
}
261+
262+
[Fact]
263+
public void ShouldKillOrphans_OnlyWithoutAnOverride()
264+
{
265+
Assert.True(GodotMcpServerPathOverride.ShouldKillOrphans(null));
266+
Assert.True(GodotMcpServerPathOverride.ShouldKillOrphans(""));
267+
268+
// The skip is a correctness requirement, not a convenience: ownership matches on the containing
269+
// directory, and an override binary is shared by design.
270+
Assert.False(GodotMcpServerPathOverride.ShouldKillOrphans("/srv/gamedev-mcp-server"));
271+
}
272+
273+
// --- "supplied, but ignored" — the state that is otherwise invisible -------------------------------
274+
275+
[Fact]
276+
public void IsIgnoredValue_TrueOnlyWhenAValueWasSuppliedAndDidNotResolve()
277+
{
278+
// Supplied, but the existence gate refused it: the addon silently downloads the pinned release,
279+
// so this is the state the boot site must warn about.
280+
Assert.True(GodotMcpServerPathOverride.IsIgnoredValue("/srv/not-built-yet", null));
281+
282+
// Nothing supplied — indistinguishable in the RESOLVED value, which is exactly why the raw value
283+
// is carried separately; it must NOT produce a warning.
284+
Assert.False(GodotMcpServerPathOverride.IsIgnoredValue(null, null));
285+
Assert.False(GodotMcpServerPathOverride.IsIgnoredValue("", null));
286+
287+
// Supplied and resolved: the override is in force, nothing to warn about.
288+
Assert.False(GodotMcpServerPathOverride.IsIgnoredValue("/srv/built", "/srv/built"));
289+
}
290+
291+
// --- what the manager LAUNCHES (GodotMcpServerManager.ExecutableFullPath) --------------------------
292+
293+
[Fact]
294+
public void ExecutablePath_OverrideActive_LaunchesTheOverride()
295+
{
296+
var overridePath = ExePath("chain", "gamedev-mcp-server.exe");
297+
var cached = ExePath("cache", "win-x64", "gamedev-mcp-server.exe");
298+
299+
Assert.Equal(overridePath, GodotMcpServerPathOverride.ExecutablePath(overridePath, cached));
300+
}
301+
302+
[Fact]
303+
public void ExecutablePath_NoOverride_LaunchesTheCachedBinary()
304+
{
305+
var cached = ExePath("cache", "win-x64", "gamedev-mcp-server.exe");
306+
307+
// Asserted first so that "the predicate is stuck on" and "the launch path ignores the predicate"
308+
// fail on DIFFERENT lines with different text, rather than both landing on the Equal below.
309+
Assert.False(GodotMcpServerPathOverride.IsActive(null));
310+
311+
Assert.Equal(cached, GodotMcpServerPathOverride.ExecutablePath(null, cached));
312+
Assert.Equal(cached, GodotMcpServerPathOverride.ExecutablePath("", cached));
313+
}
314+
315+
// --- the child process's working directory ---------------------------------------------------------
316+
317+
[Fact]
318+
public void WorkingDirectory_IsTheDirectoryOfTheResolvedExecutable()
319+
{
320+
var overrideDir = ExePath("chain", "win-x64");
321+
var overrideExe = Path.Combine(overrideDir, "gamedev-mcp-server.exe");
322+
var cacheDir = ExePath("cache", "win-x64");
323+
324+
// Not the cache folder: an override binary must run beside ITS OWN sidecar files.
325+
Assert.Equal(overrideDir, GodotMcpServerPathOverride.WorkingDirectory(overrideExe, cacheDir));
326+
}
327+
328+
[Fact]
329+
public void WorkingDirectory_NoOverride_IsStillTheCacheFolder()
330+
{
331+
var cacheDir = ExePath("cache", "win-x64");
332+
var cachedExe = Path.Combine(cacheDir, "gamedev-mcp-server.exe");
333+
334+
// In PRODUCTION the manager passes CachePlatformPath() as the fallback, so with no override both
335+
// arms of this method return the same string and the assertion could not fail. Passing a fallback
336+
// the answer must NOT be is what gives the test discriminating power: the returned cacheDir can
337+
// then only have come from the executable path, so a mutation that always returns the fallback
338+
// reddens here. The claim is unchanged — with no override the answer is the cache platform folder.
339+
var fallbackThatMustNotBeUsed = ExePath("fallback-never-used");
340+
341+
Assert.Equal(cacheDir, GodotMcpServerPathOverride.WorkingDirectory(cachedExe, fallbackThatMustNotBeUsed));
342+
}
343+
344+
[Theory]
345+
[InlineData(null)]
346+
[InlineData("")]
347+
[InlineData("gamedev-mcp-server")]
348+
public void WorkingDirectory_PathWithoutADirectoryComponent_FallsBack(string? executablePath)
349+
{
350+
var cacheDir = ExePath("cache", "win-x64");
351+
352+
Assert.Equal(cacheDir, GodotMcpServerPathOverride.WorkingDirectory(executablePath, cacheDir));
353+
}
354+
355+
// --- misuse ----------------------------------------------------------------------------------------
356+
357+
[Fact]
358+
public void Resolve_NullFileExistsDelegate_Throws()
359+
{
360+
Assert.Throws<ArgumentNullException>(
361+
() => GodotMcpServerPathOverride.Resolve("/srv/gamedev-mcp-server", null!));
362+
}
363+
}
364+
}

0 commit comments

Comments
 (0)