From 24747858eb666cd18c8e6c869093b7a5aa6dab9e Mon Sep 17 00:00:00 2001 From: EugeneTes Date: Thu, 8 Oct 2026 12:46:32 +0000 Subject: [PATCH] fix(a2a-client): fake MCP server on the ASP.NET framework, question-first results, safer file names --- .../A2AResults.cs | 46 ++++++++++++++----- .../AgentsFile.cs | 2 +- .../PendingFile.cs | 14 ++++-- ...Core.Extensions.Tests.FakeMcpServer.csproj | 1 + .../A2A/A2AResultsTests.cs | 32 ++++++++++++- .../A2A/AgentsFileTests.cs | 7 +++ .../A2A/PendingFileTests.cs | 10 ++++ ...lcanic.AgenticCore.Extensions.Tests.csproj | 8 ---- 8 files changed, 95 insertions(+), 25 deletions(-) diff --git a/extensions/Volcanic.AgenticCore.Extensions.A2A/A2AResults.cs b/extensions/Volcanic.AgenticCore.Extensions.A2A/A2AResults.cs index 6437332..d27edb0 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.A2A/A2AResults.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.A2A/A2AResults.cs @@ -4,6 +4,12 @@ using A2AMessage = global::A2A.Message; namespace Volcanic.AgenticCore.Extensions.A2A; +/// Parts not saved because they were over the size limit. +internal sealed record SkippedFiles(IReadOnlyList Names, long MaxBytes); + +/// What did: workspace-relative paths saved, and parts skipped for size. +internal sealed record SaveResult(IReadOnlyList Saved, SkippedFiles Skipped); + /// How remote tasks read for the model: state names, the answer text, saved files, result and delivery texts. internal static class A2AResults { @@ -44,17 +50,19 @@ internal static class A2AResults } /// Saves file parts (inline bytes) under inbox/a2a/<agent>/; returns workspace-relative paths. - public static IReadOnlyList SaveFiles(string workspace, string agent, AgentTask task, long maxBytes) + public static SaveResult SaveFiles(string workspace, string agent, AgentTask task, long maxBytes) { var saved = new List(); + var skipped = new List(); foreach (var artifact in task.Artifacts ?? []) foreach (var part in artifact.Parts ?? []) { - if (part.ContentCase != PartContentCase.Raw || part.Raw is not { } bytes || bytes.Length > maxBytes) continue; + if (part.ContentCase != PartContentCase.Raw || part.Raw is not { } bytes) continue; var name = Sanitize(part.Filename ?? artifact.Name ?? artifact.ArtifactId); + if (bytes.Length > maxBytes) { skipped.Add(name); continue; } var relative = $"inbox/a2a/{agent}/{name}"; var full = Path.Combine(workspace, relative); - for (var n = 2; File.Exists(full); n++) + for (var n = 2; File.Exists(full) || Directory.Exists(full); n++) { relative = $"inbox/a2a/{agent}/{Path.GetFileNameWithoutExtension(name)}-{n}{Path.GetExtension(name)}"; full = Path.Combine(workspace, relative); @@ -63,7 +71,7 @@ internal static class A2AResults File.WriteAllBytes(full, bytes); saved.Add(relative); } - return saved; + return new SaveResult(saved, new SkippedFiles(skipped, maxBytes)); } /// Links the remote agent returned instead of bytes (not downloaded). @@ -72,40 +80,54 @@ internal static class A2AResults .Select(p => p.Url!).ToList(); /// The tool result for a task. - public static string Describe(string agent, AgentTask task, IReadOnlyList files) + public static string Describe(string agent, AgentTask task, IReadOnlyList files, SkippedFiles? skipped = null) { var sb = new StringBuilder(); sb.Append($"Agent \"{agent}\": task {task.Id} (context {task.ContextId}) is {StateName(task.Status.State)}."); - AppendBody(sb, task, files); + AppendBody(sb, task, files, skipped); return sb.ToString(); } /// The follow-up prompt a watch delivers. - public static string DeliveryText(string agent, AgentTask task, IReadOnlyList files) + public static string DeliveryText(string agent, AgentTask task, IReadOnlyList files, SkippedFiles? skipped = null) { var kind = task.Status.State is TaskState.InputRequired or TaskState.AuthRequired ? "question" : "result"; var sb = new StringBuilder($"[A2A {kind} from \"{agent}\", task {task.Id} ({StateName(task.Status.State)})]"); sb.Append($"\nContext: {task.ContextId}"); - AppendBody(sb, task, files); + AppendBody(sb, task, files, skipped); return sb.ToString(); } - private static void AppendBody(StringBuilder sb, AgentTask task, IReadOnlyList files) + private static void AppendBody(StringBuilder sb, AgentTask task, IReadOnlyList files, SkippedFiles? skipped) { - if (FinalText(task) is { } text) sb.Append('\n').Append(text); + var waiting = task.Status.State is TaskState.InputRequired or TaskState.AuthRequired; + // Waiting for us: the question (status message) leads; artifacts may be stale output of earlier turns. + var text = waiting ? WaitingText(task) : FinalText(task); + if (text is not null) sb.Append('\n').Append(text); if (files.Count > 0) sb.Append("\nFiles: ").Append(string.Join(", ", files)); + if (skipped is { Names.Count: > 0 }) + sb.Append($"\nFiles skipped (over {skipped.MaxBytes} bytes): ").Append(string.Join(", ", skipped.Names)); var links = Links(task); if (links.Count > 0) sb.Append("\nLinks: ").Append(string.Join(", ", links)); if (task.Status.State is TaskState.InputRequired or TaskState.AuthRequired) sb.Append($"\nThe agent is waiting for an answer: reply with action=send, agent, taskId={task.Id} and your answer."); } + private static string? WaitingText(AgentTask task) + { + var question = TextOf(task.Status.Message); + var rest = string.Join("\n", (task.Artifacts ?? []).Select(a => TextOf(a.Parts)).Where(t => t.Length > 0)); + var all = string.Join("\n", new[] { question, rest }.Where(t => t.Length > 0)); + return all.Length > 0 ? all : null; + } + private static string Sanitize(string? name) { var file = Path.GetFileName(name ?? ""); - if (string.IsNullOrWhiteSpace(file)) return "file"; + if (string.IsNullOrWhiteSpace(file) || file is "." or "..") return "file"; var invalid = Path.GetInvalidFileNameChars(); - var clean = new string(file.Select(c => invalid.Contains(c) || char.IsWhiteSpace(c) ? '_' : c).ToArray()); + var clean = new string(file.Select(c => invalid.Contains(c) || char.IsWhiteSpace(c) ? '_' : c).Where(c => !char.IsControl(c)).ToArray()); + if (clean.Length == 0 || clean is "." or "..") return "file"; return clean.Length > 80 ? clean[^80..] : clean; } } diff --git a/extensions/Volcanic.AgenticCore.Extensions.A2A/AgentsFile.cs b/extensions/Volcanic.AgenticCore.Extensions.A2A/AgentsFile.cs index 43a6121..f913802 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.A2A/AgentsFile.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.A2A/AgentsFile.cs @@ -14,7 +14,7 @@ public sealed record AgentsFileResult(IReadOnlyList Agents, IR /// public static partial class AgentsFile { - [GeneratedRegex("^[a-z0-9-]{1,40}$", RegexOptions.CultureInvariant)] + [GeneratedRegex("^[a-z0-9-]{1,40}\\z", RegexOptions.CultureInvariant)] private static partial Regex NamePattern(); public static string PathFor(string workspace) => Path.Combine(workspace, ".volcanic", "a2a", "agents.json"); diff --git a/extensions/Volcanic.AgenticCore.Extensions.A2A/PendingFile.cs b/extensions/Volcanic.AgenticCore.Extensions.A2A/PendingFile.cs index 6fd3d7c..a032978 100644 --- a/extensions/Volcanic.AgenticCore.Extensions.A2A/PendingFile.cs +++ b/extensions/Volcanic.AgenticCore.Extensions.A2A/PendingFile.cs @@ -47,8 +47,16 @@ public static class PendingFile var path = PathFor(workspace); Directory.CreateDirectory(Path.GetDirectoryName(path)!); var temp = path + "." + Guid.NewGuid().ToString("N") + ".tmp"; - File.WriteAllText(temp, JsonSerializer.Serialize(list, Json)); - File.Move(temp, path, overwrite: true); + try + { + File.WriteAllText(temp, JsonSerializer.Serialize(list, Json)); + File.Move(temp, path, overwrite: true); + } + catch + { + try { File.Delete(temp); } catch (IOException) { } catch (UnauthorizedAccessException) { } + throw; + } } } @@ -57,7 +65,7 @@ public static class PendingFile var path = PathFor(workspace); try { - return File.Exists(path) ? JsonSerializer.Deserialize>(File.ReadAllText(path), Json) ?? [] : []; + return File.Exists(path) ? JsonSerializer.Deserialize>(File.ReadAllText(path), Json)?.OfType().ToList() ?? [] : []; } catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) { diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer.csproj b/tests/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer.csproj index 8066bd0..b085d0b 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer.csproj +++ b/tests/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer/Volcanic.AgenticCore.Extensions.Tests.FakeMcpServer.csproj @@ -6,6 +6,7 @@ Test helper: a stdio MCP server whose behaviour FAKE_MCP_MODE selects. + diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/A2AResultsTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/A2AResultsTests.cs index 1279892..73a66cb 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/A2AResultsTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/A2AResultsTests.cs @@ -45,7 +45,7 @@ public sealed class A2AResultsTests : IDisposable ArtifactId = "f1", Name = "plan.pdf", Parts = [Part.FromRaw(Encoding.UTF8.GetBytes("pdf!"), "application/pdf", "../../plan.pdf")], }); var files = A2AResults.SaveFiles(_ws, "travel-bot", task, 1024); - var file = Assert.Single(files); + var file = Assert.Single(files.Saved); Assert.Equal("inbox/a2a/travel-bot/plan.pdf", file); Assert.Equal("pdf!", File.ReadAllText(Path.Combine(_ws, file))); } @@ -71,5 +71,35 @@ public sealed class A2AResultsTests : IDisposable Assert.Contains("Files: inbox/a2a/travel-bot/x.pdf", text); } + [Fact] + public void Describe_InputRequired_QuestionComesBeforeStaleResponseArtifact() + { + var task = Task_(TaskState.InputRequired, "Which date?", + new Artifact { ArtifactId = "response", Parts = [Part.FromText("old answer")] }); + var text = A2AResults.Describe("travel-bot", task, []); + Assert.True(text.IndexOf("Which date?", StringComparison.Ordinal) < text.IndexOf("old answer", StringComparison.Ordinal)); + } + + [Fact] + public void SaveFiles_DotDotName_BecomesFile_AndControlCharsStripped() + { + var task = Task_(TaskState.Completed, null, + new Artifact { ArtifactId = "a", Parts = [Part.FromRaw(Encoding.UTF8.GetBytes("x"), "text/plain", "..")] }, + new Artifact { ArtifactId = "b", Parts = [Part.FromRaw(Encoding.UTF8.GetBytes("y"), "text/plain", "a\u0001b.txt")] }); + var files = A2AResults.SaveFiles(_ws, "bot", task, 1024).Saved; + Assert.Equal(["inbox/a2a/bot/file", "inbox/a2a/bot/ab.txt"], files); + } + + [Fact] + public void SaveFiles_ReportsOversizedParts_AndDescribeListsThem() + { + var task = Task_(TaskState.Completed, null, + new Artifact { ArtifactId = "a", Parts = [Part.FromRaw(new byte[100], "text/plain", "big.bin")] }); + var result = A2AResults.SaveFiles(_ws, "bot", task, 10); + Assert.Empty(result.Saved); + Assert.Equal(["big.bin"], result.Skipped.Names); + Assert.Contains("Files skipped (over 10 bytes): big.bin", A2AResults.Describe("bot", task, result.Saved, result.Skipped)); + } + public void Dispose() { try { Directory.Delete(_ws, true); } catch (IOException) { } } } diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/AgentsFileTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/AgentsFileTests.cs index 4150125..3b8723e 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/AgentsFileTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/AgentsFileTests.cs @@ -46,6 +46,13 @@ public sealed class AgentsFileTests : IDisposable Assert.DoesNotContain(r.Problems, p => p.Contains("secret")); } + [Fact] + public void Read_NameWithTrailingNewline_Rejected() + { + Write("""[ { "name": "a\n", "url": "https://a.example/" } ]"""); + Assert.Empty(AgentsFile.Read(_ws).Agents); + } + [Fact] public void Agents_FileEdited_AppliesToNextCall() { diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/PendingFileTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/PendingFileTests.cs index 86b5299..dbc27a1 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/PendingFileTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/A2A/PendingFileTests.cs @@ -22,6 +22,16 @@ public sealed class PendingFileTests : IDisposable Assert.Equal("b", Assert.Single(PendingFile.Read(_ws)).WatchId); } + [Fact] + public void Read_NullElements_Filtered() + { + Directory.CreateDirectory(Path.GetDirectoryName(PendingFile.PathFor(_ws))!); + File.WriteAllText(PendingFile.PathFor(_ws), "[null]"); + Assert.Empty(PendingFile.Read(_ws)); + PendingFile.Upsert(_ws, Record("a")); + Assert.Single(PendingFile.Read(_ws)); + } + [Fact] public void Read_MissingOrMalformed_Empty() { diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/Volcanic.AgenticCore.Extensions.Tests.csproj b/tests/Volcanic.AgenticCore.Extensions.Tests/Volcanic.AgenticCore.Extensions.Tests.csproj index a9b872e..ba834f8 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/Volcanic.AgenticCore.Extensions.Tests.csproj +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/Volcanic.AgenticCore.Extensions.Tests.csproj @@ -29,12 +29,4 @@ - - - - - - -