Skip to content

fix(execute_code): allow using directives at the top of a snippet #1370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lgarczyn wants to merge 2 commits into
base: beta
Choose a base branch
from
Open

fix(execute_code): allow using directives at the top of a snippet #1370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3722415
fix(execute_code): allow using directives at the top of a snippet
lgarczyn Sep 1, 2026
2fbb7a1
fix(execute_code): handle comments in directive scanning, and stop ho…
lgarczyn Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 119 additions & 19 deletions MCPForUnity/Editor/Tools/ExecuteCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using System.Reflection;
using System.Text;
using System.Text.RegularExpressions;
using MCPForUnity.Editor.Helpers;
using MCPForUnity.Runtime.Helpers;
using Microsoft.CSharp;
Expand All @@ -19,7 +20,6 @@ public static class ExecuteCode
private const int MaxCodeLength = 50000;
private const int MaxHistoryEntries = 50;
private const int MaxHistoryCodePreview = 500;
internal const int WrapperLineOffset = 10;
private const string WrapperClassName = "MCPDynamicCode";
private const string WrapperMethodName = "Execute";

Expand Down Expand Up @@ -201,7 +201,7 @@ private static object HandleReplay(JObject @params)

private static object CompileAndExecute(string code, string compiler)
{
string wrappedSource = WrapUserCode(code);
string wrappedSource = WrapUserCode(code, out int lineOffset);

string cacheKey = compiler + "\n" + wrappedSource;
if (_compiledCache.TryGetValue(cacheKey, out CompiledSnippet cached))
Expand All @@ -217,14 +217,14 @@ private static object CompileAndExecute(string code, string compiler)
case "roslyn":
if (!RoslynCompiler.IsAvailable)
return new ErrorResponse("Roslyn (Microsoft.CodeAnalysis) is not available. Install it via NuGet or use compiler='codedom'.");
compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, out var roslynErrors);
compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, lineOffset, out var roslynErrors);
if (compiled == null)
return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(roslynErrors), compiler = "roslyn" });
usedCompiler = "roslyn";
break;

case "codedom":
compiled = CodeDomCompile(wrappedSource, assemblyPaths, out var codedomErrors);
compiled = CodeDomCompile(wrappedSource, assemblyPaths, lineOffset, out var codedomErrors);
if (compiled == null)
return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(codedomErrors), compiler = "codedom" });
usedCompiler = "codedom";
Expand All @@ -233,14 +233,14 @@ private static object CompileAndExecute(string code, string compiler)
default: // "auto"
if (RoslynCompiler.IsAvailable)
{
compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, out var autoErrors);
compiled = RoslynCompiler.Compile(wrappedSource, assemblyPaths, lineOffset, out var autoErrors);
if (compiled == null)
return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(autoErrors), compiler = "roslyn" });
usedCompiler = "roslyn";
}
else
{
compiled = CodeDomCompile(wrappedSource, assemblyPaths, out var autoFallbackErrors);
compiled = CodeDomCompile(wrappedSource, assemblyPaths, lineOffset, out var autoFallbackErrors);
if (compiled == null)
return new ErrorResponse("Compilation failed", new { errors = OffsetErrors(autoFallbackErrors), compiler = "codedom" });
usedCompiler = "codedom";
Expand Down Expand Up @@ -302,7 +302,7 @@ private static List<string> OffsetErrors(List<string> errors)

// ──────────────────── CodeDom compiler ────────────────────

private static Assembly CodeDomCompile(string source, string[] assemblyPaths, out List<string> errors)
private static Assembly CodeDomCompile(string source, string[] assemblyPaths, int lineOffset, out List<string> errors)
{
errors = new List<string>();

Expand Down Expand Up @@ -360,7 +360,7 @@ private static Assembly CodeDomCompile(string source, string[] assemblyPaths, ou
continue;

hasRealErrors = true;
int userLine = Math.Max(1, error.Line - WrapperLineOffset);
int userLine = Math.Max(1, error.Line - lineOffset);
errors.Add($"Line {userLine}: {error.ErrorText}");
}

Expand Down Expand Up @@ -508,22 +508,80 @@ public CodeDomAssemblyCandidate(string path, AssemblyName assemblyName)

// ──────────────────── Shared helpers ────────────────────

private static string WrapUserCode(string code)
private static readonly string[] BuiltinUsings =
{
"using System;",
"using System.Collections.Generic;",
"using System.Linq;",
"using System.Reflection;",
"using System.Text;",
"using UnityEngine;",
"using UnityEditor;",
};

private static readonly Regex UsingDirectiveLine = new Regex(
@"^\s*using\s+(?:static\s+)?[A-Za-z_]\w*(?:\s*\.\s*[A-Za-z_]\w*)*\s*;\s*$", RegexOptions.Compiled);

// Must not match the C# 8 `using var x = ...` declaration, which has two tokens before the =
private static readonly Regex UsingAliasLine = new Regex(
@"^\s*using\s+[A-Za-z_]\w*\s*=\s*[A-Za-z_][\w.]*\s*(?:<[^;]*>)?\s*(?:\[\s*\])*\s*;\s*$", RegexOptions.Compiled);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private static bool IsUsingDirective(string line)
=> UsingDirectiveLine.IsMatch(line) || UsingAliasLine.IsMatch(line);

// A directive can carry comments, but the regexes above must see a bare `using ...;`
private static string StripComments(string line, ref bool inBlock)
{
var sb = new StringBuilder();
sb.AppendLine("using System;");
sb.AppendLine("using System.Collections.Generic;");
sb.AppendLine("using System.Linq;");
sb.AppendLine("using System.Reflection;");
sb.AppendLine("using UnityEngine;");
sb.AppendLine("using UnityEditor;");
for (int i = 0; i < line.Length; i++)
{
bool pair = i + 1 < line.Length;
if (inBlock)
{
if (pair && line[i] == '*' && line[i + 1] == '/') { inBlock = false; i++; }
continue;
}
if (pair && line[i] == '/' && line[i + 1] == '*') { inBlock = true; i++; continue; }
if (pair && line[i] == '/' && line[i + 1] == '/') break;
sb.Append(line[i]);
}
return sb.ToString();
}

// Blanked, not removed, so compiler line numbers still map to the caller's
private static List<string> HoistLeadingUsings(string[] body)
{
var hoisted = new List<string>();
bool inBlock = false;
for (int i = 0; i < body.Length; i++)
{
string directive = StripComments(body[i], ref inBlock).Trim();
if (directive.Length == 0) continue;
if (!IsUsingDirective(directive)) break;
if (!BuiltinUsings.Contains(directive) && !hoisted.Contains(directive)) hoisted.Add(directive);
body[i] = string.Empty;
}
return hoisted;
}

// A using directive is illegal in a method body, so the parser reads it as a using-statement and wants a '('
private static string WrapUserCode(string code, out int lineOffset)
{
var body = code.Replace("\r\n", "\n").Split('\n');
var hoisted = HoistLeadingUsings(body);

var sb = new StringBuilder();
foreach (string u in BuiltinUsings) sb.AppendLine(u);
foreach (string u in hoisted) sb.AppendLine(u);
sb.AppendLine($"public static class {WrapperClassName}");
sb.AppendLine("{");
sb.AppendLine($" public static object {WrapperMethodName}()");
sb.AppendLine(" {");
sb.AppendLine(code);
sb.AppendLine(string.Join("\n", body));
sb.AppendLine(" }");
sb.AppendLine("}");
// 4 = the class line, its brace, the method line, its brace
lineOffset = BuiltinUsings.Length + hoisted.Count + 4;
return sb.ToString();
}

Expand Down Expand Up @@ -559,9 +617,50 @@ private static string[] ResolveAssemblyPaths()
return result;
}

// A hoisted `using System.IO;` puts File.Delete in scope, which no fully-qualified
// pattern in _blockedPatterns matches. Derive the short forms the directives create.
private static IEnumerable<string> ShortFormsFromUsings(IEnumerable<string> directives)
{
foreach (string directive in directives)
{
string target = directive.Substring("using".Length).Trim().TrimEnd(';').Trim();
bool isStatic = target.StartsWith("static ", StringComparison.Ordinal);
if (isStatic) target = target.Substring("static ".Length).Trim();

string alias = null;
int eq = target.IndexOf('=');
if (eq >= 0)
{
alias = target.Substring(0, eq).Trim();
target = target.Substring(eq + 1).Trim();
}

int lastDot = target.LastIndexOf('.');
string leaf = lastDot >= 0 ? target.Substring(lastDot + 1) : target;

foreach (string pattern in _blockedPatterns)
{
if (pattern.StartsWith(target + ".", StringComparison.OrdinalIgnoreCase))
{
string tail = pattern.Substring(target.Length + 1);
yield return alias == null ? tail : alias + "." + tail;
}
// A blocked pattern already written short, like AssetDatabase.DeleteAsset,
// needs the leaf instead: `using static ...Process;` makes Process.Start bare Start
else if (pattern.StartsWith(leaf + ".", StringComparison.OrdinalIgnoreCase))
{
string tail = pattern.Substring(leaf.Length + 1);
if (isStatic) yield return tail;
else if (alias != null) yield return alias + "." + tail;
}
}
}
}

private static string CheckBlockedPatterns(string code)
{
foreach (var pattern in _blockedPatterns)
var directives = HoistLeadingUsings(code.Replace("\r\n", "\n").Split('\n'));
foreach (var pattern in _blockedPatterns.Concat(ShortFormsFromUsings(directives)))
{
if (code.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) >= 0)
return $"Code contains blocked pattern: '{pattern}'. Disable safety checks with safety_checks=false if this is intentional.";
Expand Down Expand Up @@ -663,6 +762,7 @@ public static void ResetCache()
_isAvailable = null;
}


private static bool Initialize()
{
try
Expand Down Expand Up @@ -752,7 +852,7 @@ private static bool Initialize()
}
}

public static Assembly Compile(string source, string[] assemblyPaths, out List<string> errors)
public static Assembly Compile(string source, string[] assemblyPaths, int lineOffset, out List<string> errors)
{
errors = new List<string>();

Expand Down Expand Up @@ -837,7 +937,7 @@ public static Assembly Compile(string source, string[] assemblyPaths, out List<s
var msgProp = diag.GetType().GetMethod("GetMessage", new[] { typeof(System.Globalization.CultureInfo) });
string msg = (string)msgProp.Invoke(diag, new object[] { null });

int userLine = Math.Max(1, line + 1 - ExecuteCode.WrapperLineOffset);
int userLine = Math.Max(1, line + 1 - lineOffset);
errors.Add($"Line {userLine}: {msg}");
}
return null;
Expand Down
115 changes: 115 additions & 0 deletions TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecuteCodeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,121 @@ private static void AssertCompilerSuccess(CompilerResults results)
Assert.IsFalse(results.Errors.HasErrors, string.Join("\n", errors));
}


// ──────────────────── Execute: using directives ────────────────────

[Test]
public void Execute_LeadingUsingDirective_IsHoistedAndCompiles()
{
var result = Execute("using System.IO;\nreturn Path.GetFileName(\"/a/b.txt\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("b.txt", result["data"]["result"].Value<string>());
}

[Test]
public void Execute_UsingStaticDirective_IsHoisted()
{
var result = Execute("using static System.Math;\nreturn Max(3, 4);");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual(4, result["data"]["result"].Value<int>());
}

[Test]
public void Execute_UsingAliasWithGenericTarget_IsHoisted()
{
var result = Execute("using L = System.Collections.Generic.List<int>;\nreturn new L { 1, 2 }.Count;");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual(2, result["data"]["result"].Value<int>());
}

[Test]
public void Execute_UsingAfterComment_IsStillHoisted()
{
var result = Execute("// leading comment\nusing System.IO;\nreturn Path.GetFileName(\"/x/y.txt\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("y.txt", result["data"]["result"].Value<string>());
}


[Test]
public void Execute_RedundantBuiltinUsing_DoesNotDuplicate()
{
var result = Execute("using System;\nreturn String.Concat(\"a\", \"b\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("ab", result["data"]["result"].Value<string>());
}

[Test]
public void Execute_ErrorLineNumber_CountsFromCallerCodeAfterHoisting()
{
var result = Execute("using System.IO;\nint ok = 1;\nnope();\nreturn ok;");

Assert.IsFalse(result.Value<bool>("success"), result.ToString());
var errors = string.Join("\n", (result["data"]["errors"] as JArray).Select(e => e.Value<string>()));
StringAssert.Contains("Line 3", errors);
}

[Test]
public void Execute_UsingWithTrailingComment_IsHoisted()
{
var result = Execute("using System.IO; // needed for Path\nreturn Path.GetFileName(\"/a/b.txt\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("b.txt", result["data"]["result"].Value<string>());
}

[Test]
public void Execute_UsingAfterBlockComment_IsStillHoisted()
{
var result = Execute("/* lead\n in */\nusing System.IO;\nreturn Path.GetFileName(\"/a/c.txt\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("c.txt", result["data"]["result"].Value<string>());
}

[Test]
public void Execute_UsingBehindInlineBlockComment_IsHoisted()
{
var result = Execute("/* why */ using System.IO;\nreturn Path.GetFileName(\"/a/d.txt\");");

Assert.IsTrue(result.Value<bool>("success"), result.ToString());
Assert.AreEqual("d.txt", result["data"]["result"].Value<string>());
}

// ──────────────────── Execute: hoisting must not widen safety_checks ────────────────────

[Test]
public void Execute_HoistedNamespace_DoesNotBypassSafetyChecks()
{
var result = Execute("using System.IO;\nFile.Delete(\"/tmp/mcp-does-not-exist\");\nreturn 1;");

Assert.IsFalse(result.Value<bool>("success"), result.ToString());
StringAssert.Contains("Blocked pattern", result.Value<string>("error"));
}

[Test]
public void Execute_AliasedType_DoesNotBypassSafetyChecks()
{
var result = Execute("using F = System.IO.File;\nF.Delete(\"/tmp/mcp-does-not-exist\");\nreturn 1;");

Assert.IsFalse(result.Value<bool>("success"), result.ToString());
StringAssert.Contains("Blocked pattern", result.Value<string>("error"));
}

[Test]
public void Execute_UsingStatic_DoesNotBypassSafetyChecks()
{
var result = Execute("using static System.Diagnostics.Process;\nStart(\"ls\");\nreturn 1;");

Assert.IsFalse(result.Value<bool>("success"), result.ToString());
StringAssert.Contains("Blocked pattern", result.Value<string>("error"));
}

private static JObject Execute(string code)
{
return ToJObject(ExecuteCode.HandleCommand(new JObject
Expand Down
Loading