diff --git a/OpenNest.FrontEnd.Tests/McpEngineHarnessProcessTests.cs b/OpenNest.FrontEnd.Tests/McpEngineHarnessProcessTests.cs new file mode 100644 index 0000000..bf1cbc7 --- /dev/null +++ b/OpenNest.FrontEnd.Tests/McpEngineHarnessProcessTests.cs @@ -0,0 +1,350 @@ +using System.Diagnostics; +using System.Text.Json; +using Microsoft.Extensions.Configuration; +using OpenNest.Mcp.Tools; + +namespace OpenNest.FrontEnd.Tests; + +public class McpEngineHarnessProcessTests(McpEngineHarnessChildFixture fixture) + : IClassFixture +{ + private static readonly TimeSpan Watchdog = TimeSpan.FromSeconds(30); + + [Fact] + public async Task Normal_ReachesPrivateChild() + { + var root = fixture.NewRun(); + var result = await fixture.Tool().TestEngine(root).WaitAsync(Watchdog); + await McpEngineHarnessChildFixture.Ready(root); + Assert.Equal(JsonSerializer.Serialize(fixture.Arguments(root)), result); + } + + [Fact] + public async Task Completed_PreservesStderrAndNonzeroStatus() + { + var root = fixture.NewRun(); + var result = await fixture.Tool().TestEngine(root, "nonzero").WaitAsync(Watchdog); + Assert.Equal(JsonSerializer.Serialize(fixture.Arguments(root, "nonzero")) + + Environment.NewLine + Environment.NewLine + "=== Errors ===" + Environment.NewLine + + "fixture stderr" + Environment.NewLine + "Process exited with code 7" + Environment.NewLine, result); + } + + [Fact] + public async Task Arguments_PreserveSpacesAndQuotes() + { + var root = fixture.NewRun(); + const string drawing = "drawing with \"quotes\" and trailing\\"; + var output = Path.Combine(fixture.Directory, "output with spaces.nest"); + var result = await fixture.Tool().TestEngine(root, drawing, 42, output).WaitAsync(Watchdog); + Assert.Equal(fixture.Arguments(root, drawing, 42, output), JsonSerializer.Deserialize(result)); + } + + [Fact] + public async Task ConcurrentDrains_PreserveFullOutputBeyondPipeCapacity() + { + var root = fixture.NewRun(); + var result = await fixture.Tool().TestEngine(root, "flood").WaitAsync(Watchdog); + Assert.Equal(JsonSerializer.Serialize(fixture.Arguments(root, "flood")) + Environment.NewLine + + new string('O', 256 * 1024) + Environment.NewLine + Environment.NewLine + + "=== Errors ===" + Environment.NewLine + new string('E', 256 * 1024), result); + } + + [Fact] + public async Task Deadline_BoundsHungChildAndReapsIt() + { + var root = fixture.NewRun(); + var run = fixture.Tool(5).TestEngine(root, "wait"); + Process? parent = null; + try + { + await McpEngineHarnessChildFixture.Ready(root); + parent = McpEngineHarnessChildFixture.GetProcess(root, "parent"); + var result = await run.WaitAsync(Watchdog); + Assert.Contains("timed out", result, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("Cleanup failed", result); + await parent.WaitForExitAsync().WaitAsync(Watchdog); + } + finally + { + McpEngineHarnessChildFixture.Release(root); + McpEngineHarnessChildFixture.Stop(parent); + await run.WaitAsync(Watchdog); + parent?.Dispose(); + } + } + + [Fact] + public async Task Cancellation_StopsOwnedLiveTreeWithoutKillingUnrelatedProcess() + { + using var cancellation = new CancellationTokenSource(); + var root = fixture.NewRun(); + var unrelated = fixture.NewRun(); + using var independent = Process.Start(fixture.StartInfo(unrelated, "wait"))!; + var run = fixture.Tool().TestEngine(root, "tree", cancellationToken: cancellation.Token); + Process? parent = null; + Process? descendant = null; + try + { + await McpEngineHarnessChildFixture.Ready(root); + await McpEngineHarnessChildFixture.Ready(unrelated); + parent = McpEngineHarnessChildFixture.GetProcess(root, "parent"); + descendant = McpEngineHarnessChildFixture.GetProcess(root, "child"); + await cancellation.CancelAsync(); + var result = await run.WaitAsync(Watchdog); + Assert.Contains("cancelled", result, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("Cleanup failed", result); + await parent.WaitForExitAsync().WaitAsync(Watchdog); + await descendant.WaitForExitAsync().WaitAsync(Watchdog); + Assert.False(independent.HasExited); + } + finally + { + McpEngineHarnessChildFixture.Release(root); + McpEngineHarnessChildFixture.Release(unrelated); + McpEngineHarnessChildFixture.Stop(parent); + McpEngineHarnessChildFixture.Stop(descendant); + McpEngineHarnessChildFixture.Stop(independent); + await run.WaitAsync(Watchdog); + parent?.Dispose(); + descendant?.Dispose(); + } + } + + [Fact] + public async Task Deadline_BoundsPipeDrainAfterParentExits() + { + var root = fixture.NewRun(); + var run = fixture.Tool(5).TestEngine(root, "orphan"); + Process? parent = null; + Process? descendant = null; + try + { + await McpEngineHarnessChildFixture.Ready(root); + parent = McpEngineHarnessChildFixture.GetProcess(root, "parent"); + descendant = McpEngineHarnessChildFixture.GetProcess(root, "child"); + McpEngineHarnessChildFixture.ExitParent(root); + await parent.WaitForExitAsync().WaitAsync(Watchdog); + Assert.False(descendant.HasExited); + var result = await run.WaitAsync(Watchdog); + Assert.Contains("timed out", result, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("Cleanup failed", result); + } + finally + { + // A descendant detached after parent exit cannot be identified by Process.Kill(tree). + McpEngineHarnessChildFixture.Release(root); + McpEngineHarnessChildFixture.Stop(parent); + McpEngineHarnessChildFixture.Stop(descendant); + await run.WaitAsync(Watchdog); + parent?.Dispose(); + descendant?.Dispose(); + } + } + + [Fact] + public async Task AlreadyCancelled_DoesNotStartChild() + { + var root = fixture.NewRun(); + var result = await fixture.Tool().TestEngine(root, + cancellationToken: new CancellationToken(true)).WaitAsync(Watchdog); + Assert.Contains("cancelled", result, StringComparison.OrdinalIgnoreCase); + Assert.False(File.Exists(root + ".parent")); + } + + [Theory] + [InlineData("EngineHarness:SourceRoot", "relative checkout")] + [InlineData("EngineHarness:DotnetPath", "dotnet")] + [InlineData("EngineHarness:TimeoutSeconds", "0")] + [InlineData("EngineHarness:TimeoutSeconds", "2147484")] + [InlineData("EngineHarness:TimeoutSeconds", "invalid")] + public async Task InvalidConfiguration_FailsClearlyWithoutStartingChild(string key, string value) + { + var root = fixture.NewRun(); + var result = await fixture.Tool(overrideKey: key, overrideValue: value).TestEngine(root).WaitAsync(Watchdog); + Assert.Contains(key, result); + Assert.False(File.Exists(root + ".parent")); + } + + [Fact] + public async Task MissingNest_FailsClearly() + { + Assert.Contains("nest file not found", await fixture.Tool().TestEngine("missing.nest")); + } +} + +public sealed class McpEngineHarnessChildFixture : IAsyncLifetime +{ + public string Directory { get; } = Path.Combine(Path.GetTempPath(), "opennest-mcp-harness-" + Guid.NewGuid()); + public string Bin => Path.Combine(Directory, "bin"); + public string Host => Path.Combine(Bin, OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet"); + public string SourceRoot => Path.Combine(Directory, "private checkout with spaces"); + + public async Task InitializeAsync() + { + System.IO.Directory.CreateDirectory(Directory); + System.IO.Directory.CreateDirectory(Path.Combine(SourceRoot, "OpenNest.Console")); + await File.WriteAllTextAsync(Path.Combine(SourceRoot, "OpenNest.Console", "OpenNest.Console.csproj"), ""); + var project = Path.Combine(Directory, "HarnessChild.csproj"); + await File.WriteAllTextAsync(project, """ + + + Exe + net8.0 + enable + false + + + + """); + await File.WriteAllTextAsync(Path.Combine(Directory, "Program.cs"), ChildSource); + var runtime = System.Runtime.InteropServices.RuntimeEnvironment.GetRuntimeDirectory(); + var dotnet = Path.GetFullPath(Path.Combine(runtime, "..", "..", "..", + OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet")); + var info = new ProcessStartInfo(dotnet) + { + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + foreach (var arg in new[] { "build", project, "--output", Bin, "--nologo" }) + info.ArgumentList.Add(arg); + using var process = Process.Start(info)!; + var stdout = process.StandardOutput.ReadToEndAsync(); + var stderr = process.StandardError.ReadToEndAsync(); + try + { + await process.WaitForExitAsync().WaitAsync(TimeSpan.FromSeconds(120)); + Assert.True(process.ExitCode == 0, await stdout + await stderr); + } + finally + { + Stop(process); + } + File.Copy(Path.Combine(Bin, OperatingSystem.IsWindows() ? "HarnessChild.exe" : "HarnessChild"), Host); + } + + public Task DisposeAsync() + { + System.IO.Directory.Delete(Directory, true); + return Task.CompletedTask; + } + + public string NewRun() + { + var path = Path.Combine(Directory, "run " + Guid.NewGuid() + ".nest"); + File.WriteAllText(path, "process control fixture, not a real nest"); + return path; + } + + public TestTools Tool(int timeoutSeconds = 120, string? overrideKey = null, string? overrideValue = null) + { + var values = new Dictionary + { + ["EngineHarness:SourceRoot"] = SourceRoot, + ["EngineHarness:DotnetPath"] = Host, + ["EngineHarness:TimeoutSeconds"] = timeoutSeconds.ToString(), + }; + if (overrideKey != null) values[overrideKey] = overrideValue; + return new TestTools(new ConfigurationBuilder().AddInMemoryCollection(values).Build()); + } + + public string[] Arguments(string run, string? drawing = null, int plateIndex = 0, string? output = null) + { + var arguments = new List + { + "run", "--project", Path.Combine(SourceRoot, "OpenNest.Console", "OpenNest.Console.csproj"), "--", run, + }; + if (drawing != null) arguments.AddRange(["--drawing", drawing]); + arguments.AddRange(["--plate", plateIndex.ToString()]); + if (output != null) arguments.AddRange(["--output", output]); + return arguments.ToArray(); + } + + public ProcessStartInfo StartInfo(string run, string mode) + { + var info = new ProcessStartInfo(Host) + { + WorkingDirectory = Directory, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + foreach (var arg in new[] { "run", "--project", "unused", "--", run, "--drawing", mode }) + info.ArgumentList.Add(arg); + return info; + } + + public static async Task Ready(string run) + { + using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + while (!File.Exists(run + ".ready")) await Task.Delay(25, timeout.Token); + } + + public static Process GetProcess(string run, string name) => System.Diagnostics.Process.GetProcessById( + int.Parse(File.ReadAllText(run + "." + name))); + + public static void ExitParent(string run) => File.WriteAllText(run + ".exit", "exit"); + + public static void Release(string run) => File.WriteAllText(run + ".release", "release"); + + public static void Stop(Process? process) + { + if (process == null || process.HasExited) return; + process.Kill(entireProcessTree: true); + Assert.True(process.WaitForExit(10_000), "Fixture child must be reaped."); + } + + private const string ChildSource = """ + using System.Diagnostics; + using System.Text.Json; + + if (args[0] == "descendant") + { + var path = args[1]; + File.WriteAllText(path + ".child", Environment.ProcessId.ToString()); + File.WriteAllText(path + ".ready", "ready"); + while (!File.Exists(path + ".release")) Thread.Sleep(25); + return 0; + } + var separator = Array.IndexOf(args, "--"); + var input = args[separator + 1]; + var drawing = Array.IndexOf(args, "--drawing"); + var mode = drawing < 0 ? "normal" : args[drawing + 1]; + if (mode == "tree" || mode == "orphan") + { + File.WriteAllText(input + ".parent", Environment.ProcessId.ToString()); + var childInfo = new ProcessStartInfo(Environment.ProcessPath!) { UseShellExecute = false }; + childInfo.ArgumentList.Add("descendant"); + childInfo.ArgumentList.Add(input); + using var child = Process.Start(childInfo)!; + if (mode == "orphan") + { + while (!File.Exists(input + ".exit")) Thread.Sleep(25); + return 0; + } + while (!File.Exists(input + ".release")) Thread.Sleep(25); + child.WaitForExit(); + return 0; + } + File.WriteAllText(input + ".parent", Environment.ProcessId.ToString()); + File.WriteAllText(input + ".ready", "ready"); + if (mode == "wait") + { + while (!File.Exists(input + ".release")) Thread.Sleep(25); + return 0; + } + Console.WriteLine(JsonSerializer.Serialize(args)); + if (mode == "nonzero") + { + Console.Error.WriteLine("fixture stderr"); + return 7; + } + if (mode == "flood") + { + var stdout = Task.Run(() => Console.Write(new string('O', 256 * 1024))); + var stderr = Task.Run(() => Console.Error.Write(new string('E', 256 * 1024))); + Task.WaitAll(stdout, stderr); + } + return 0; + """; +} diff --git a/OpenNest.Mcp/Tools/TestTools.cs b/OpenNest.Mcp/Tools/TestTools.cs index 72e7337..3c76e02 100644 --- a/OpenNest.Mcp/Tools/TestTools.cs +++ b/OpenNest.Mcp/Tools/TestTools.cs @@ -1,7 +1,13 @@ +using System; using System.ComponentModel; using System.Diagnostics; +using System.Globalization; using System.IO; +using System.Runtime.InteropServices; using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Configuration; using ModelContextProtocol.Server; namespace OpenNest.Mcp.Tools @@ -9,86 +15,149 @@ namespace OpenNest.Mcp.Tools [McpServerToolType] public class TestTools { - private const string SolutionRoot = @"C:\Users\AJ\Desktop\Projects\OpenNest"; + private readonly IConfiguration configuration; + private static readonly TimeSpan CleanupTimeout = TimeSpan.FromSeconds(5); - private static readonly string HarnessProject = Path.Combine( - SolutionRoot, - "OpenNest.Console", - "OpenNest.Console.csproj" - ); + public TestTools(IConfiguration configuration) + { + this.configuration = configuration; + } [McpServerTool(Name = "test_engine")] [Description( "Build and run the nesting engine against a nest file. Returns fill results and a debug log file path for grepping. Use this to test engine changes without restarting the MCP server." )] - public string TestEngine( - [Description("Path to the nest .nest file")] - string nestFile = @"C:\Users\AJ\Desktop\4980 A24 PT02 60x120 45pcs v2.nest", - [Description("Drawing name to fill with (default: first drawing)")] - string drawingName = null, + public async Task TestEngine( + [Description("Path to the nest .nest file")] string nestFile, + [Description("Drawing name to fill with (default: first drawing)")] string drawingName = null, [Description("Plate index to fill (default: 0)")] int plateIndex = 0, - [Description("Output nest file path (default: -result.nest)")] - string outputFile = null + [Description("Output nest file path (default: -result.nest)")] string outputFile = null, + CancellationToken cancellationToken = default ) { if (!File.Exists(nestFile)) return $"Error: nest file not found: {nestFile}"; - var processArgs = new StringBuilder(); - processArgs.Append($"\"{nestFile}\""); - - if (!string.IsNullOrEmpty(drawingName)) - processArgs.Append($" --drawing \"{drawingName}\""); - - processArgs.Append($" --plate {plateIndex}"); - - if (!string.IsNullOrEmpty(outputFile)) - processArgs.Append($" --output \"{outputFile}\""); - - var psi = new ProcessStartInfo - { - FileName = "dotnet", - Arguments = $"run --project \"{HarnessProject}\" -- {processArgs}", - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - WorkingDirectory = SolutionRoot, - }; - - var sb = new StringBuilder(); - + using var process = new Process(); + var started = false; + Task completion = null; try { - using var process = Process.Start(psi); - var stderrTask = process.StandardError.ReadToEndAsync(); - var stdout = process.StandardOutput.ReadToEnd(); - process.WaitForExit(120_000); - var stderr = stderrTask.Result; + var sourceRoot = configuration["EngineHarness:SourceRoot"]; + if (string.IsNullOrWhiteSpace(sourceRoot) || !Path.IsPathFullyQualified(sourceRoot)) + return "Error: EngineHarness:SourceRoot must be an absolute OpenNest checkout path."; + var harnessProject = Path.Combine(sourceRoot, "OpenNest.Console", "OpenNest.Console.csproj"); + if (!File.Exists(harnessProject)) + return $"Error: harness project not found: {harnessProject}"; + + var dotnetPath = configuration["EngineHarness:DotnetPath"] ?? Path.GetFullPath(Path.Combine( + RuntimeEnvironment.GetRuntimeDirectory(), "..", "..", "..", + OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet")); + if (!Path.IsPathFullyQualified(dotnetPath) || !File.Exists(dotnetPath)) + return "Error: EngineHarness:DotnetPath must identify an existing absolute dotnet executable."; + + if (!int.TryParse(configuration["EngineHarness:TimeoutSeconds"] ?? "120", + NumberStyles.None, CultureInfo.InvariantCulture, out var timeoutSeconds) + || timeoutSeconds <= 0 || timeoutSeconds > int.MaxValue / 1000) + return "Error: EngineHarness:TimeoutSeconds must be a positive finite deadline (at most 2147483 seconds)."; + + var psi = new ProcessStartInfo(dotnetPath) + { + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true, + WorkingDirectory = sourceRoot, + }; + foreach (var argument in new[] { "run", "--project", harnessProject, "--", Path.GetFullPath(nestFile) }) + psi.ArgumentList.Add(argument); + if (!string.IsNullOrEmpty(drawingName)) + { + psi.ArgumentList.Add("--drawing"); + psi.ArgumentList.Add(drawingName); + } + psi.ArgumentList.Add("--plate"); + psi.ArgumentList.Add(plateIndex.ToString(CultureInfo.InvariantCulture)); + if (!string.IsNullOrEmpty(outputFile)) + { + psi.ArgumentList.Add("--output"); + psi.ArgumentList.Add(Path.GetFullPath(outputFile)); + } + process.StartInfo = psi; + + using var deadline = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + deadline.CancelAfter(TimeSpan.FromSeconds(timeoutSeconds)); + deadline.Token.ThrowIfCancellationRequested(); + started = process.Start(); + if (!started) + return "Error running test harness: process did not start."; + + var stdoutTask = process.StandardOutput.ReadToEndAsync(deadline.Token); + var stderrTask = process.StandardError.ReadToEndAsync(deadline.Token); + completion = Task.WhenAll(process.WaitForExitAsync(deadline.Token), stdoutTask, stderrTask); + // Pipe EOF can outlive the direct process, so bound the aggregate as well. + await completion.WaitAsync(deadline.Token); + var stdout = await stdoutTask; + var stderr = await stderrTask; + + var result = new StringBuilder(); if (!string.IsNullOrWhiteSpace(stdout)) - sb.Append(stdout.TrimEnd()); - + result.Append(stdout.TrimEnd()); if (!string.IsNullOrWhiteSpace(stderr)) { - sb.AppendLine(); - sb.AppendLine(); - sb.AppendLine("=== Errors ==="); - sb.Append(stderr.TrimEnd()); + result.AppendLine(); + result.AppendLine(); + result.AppendLine("=== Errors ==="); + result.Append(stderr.TrimEnd()); } - if (process.ExitCode != 0) { - sb.AppendLine(); - sb.AppendLine($"Process exited with code {process.ExitCode}"); + result.AppendLine(); + result.AppendLine($"Process exited with code {process.ExitCode}"); } + return result.ToString(); } - catch (System.Exception ex) + catch (Exception ex) { - sb.AppendLine($"Error running test harness: {ex.Message}"); + var error = $"Error running test harness: {ex.Message}"; + if (ex is OperationCanceledException) + { + var reason = cancellationToken.IsCancellationRequested ? "cancelled" : "timed out"; + error = $"Error running test harness: {reason}."; + } + if (started) + error += await StopProcessAsync(process); + return error; } + finally + { + if (started) + { + // Close our pipe ends even if a detached descendant still owns writers. + process.StandardOutput.Dispose(); + process.StandardError.Dispose(); + } + if (completion != null) + _ = completion.ContinueWith(task => _ = task.Exception, CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + } - return sb.ToString(); + private static async Task StopProcessAsync(Process process) + { + try + { + if (!process.HasExited) + process.Kill(entireProcessTree: true); + await process.WaitForExitAsync().WaitAsync(CleanupTimeout); + return string.Empty; + } + catch (Exception ex) + { + return $" Cleanup failed: {ex.Message}"; + } } } } diff --git a/docs/automatic-nesting.md b/docs/automatic-nesting.md index 0d18c1f..1018c82 100644 --- a/docs/automatic-nesting.md +++ b/docs/automatic-nesting.md @@ -46,6 +46,37 @@ Invalid output is printed and rejected with exit code 2 without saving or postin MCP `autonest_plate` requires an empty target. The stdio server serializes all tool calls sharing its mutable session, so another request cannot change drawings or occupy a target during a solve. `allow_invalid` defaults to false. It reports violations and makes no changes on rejection, including with an override when the output is unrepresentable or contains multiple sheets. Existing fill tools remain separate. Console and MCP load jobs plug-ins from `Engines/` beside their executable. +### MCP engine development harness + +`test_engine` builds and runs `OpenNest.Console` in a configured, trusted checkout. +`nestFile` is required; there is no machine-specific sample default. Drawing, +plate and output arguments and the stdout / `=== Errors ===` / nonzero-exit +response format are unchanged. Relative nest/output paths resolve from the MCP +server's working directory, before launching the checkout's console. + +The existing .NET host configuration accepts: + +- `EngineHarness:SourceRoot`: required absolute checkout path containing + `OpenNest.Console/OpenNest.Console.csproj`. +- `EngineHarness:DotnetPath`: optional existing absolute executable path; + defaults to the dotnet host in the active .NET installation, never a PATH search. +- `EngineHarness:TimeoutSeconds`: positive integer, default **120**, maximum + 2147483. The single deadline covers build/run, process exit and both concurrent + output drains. MCP request cancellation uses the same bounded path. + +For environment variables, use `EngineHarness__SourceRoot`, +`EngineHarness__DotnetPath` and `EngineHarness__TimeoutSeconds`. These are +server/operator settings, not caller-supplied executable or checkout overrides. + +Timeout/cancellation returns a clear error, kills the owned live process tree, +waits up to five additional seconds for direct-process cleanup, and closes local +pipe readers. Cleanup failures are reported. A descendant already detached when +its parent exits cannot reliably be found by `Process.Kill(entireProcessTree)`; +it may remain alive, but inherited pipe writers cannot hold the tool open. +The harness does not scan for or kill unrelated processes. Windows process-tree +semantics still need Windows runtime acceptance; the private process regressions +also execute on Linux without customer nest files. + ## .NET API and saved responses Set `NestRequest.Engine` to a registered engine name; null retains `PlacementStrategy` / legacy `Strategy` behavior. Library hosts own plug-in discovery via `NestingEngineRegistry.LoadPlugins` before calling the API. Explicit request requirement IDs are preserved in response fulfillment even when multiple requirements use the same source DXF.