Adopted further C# 14 and .NET 10 features (#1158)

app/Tests/Analyzers/ThisUsageAnalyzerTests.cs

@@ -15,6 +15,9 @@ namespace AIStudio.Tests.Analyzers;
 /// refuses the keyword. There, every instance member is reached through the name of the receiver, and a sibling
 /// member without it does not even compile. Should a later version of C# reach the receiver without its name,
 /// the analyzer would demand a <c>this.</c> which nobody can write, and the tests for extension blocks fail.
+///
+/// The same holds for the keyword <c>field</c> of a property: its backing field has no name which could follow
+/// <c>this.</c>, while every other member next to it still needs one.
 /// </remarks>
 [TestFixture]
 public sealed class ThisUsageAnalyzerTests
@@ -46,6 +49,28 @@ public sealed class ThisUsageAnalyzerTests
         }
     }
 
+    [TestCase("get => field ?? this.Name; set => field = value;", TestName = "A getter and a setter which reach the backing field")]
+    [TestCase("get; set => field = value.Trim();", TestName = "A setter which reaches the backing field")]
+    [TestCase("get => field ??= this.Describe();", TestName = "A getter which fills the backing field on first use")]
+    public async Task TheBackingFieldOfAPropertyIsLeftAlone(string accessors)
+    {
+        var reported = await AnalyzePropertyAsync(accessors);
+
+        Assert.That(reported, Is.Empty);
+    }
+
+    [Test]
+    public async Task AMemberNextToTheBackingFieldStillNeedsThis()
+    {
+        var reported = await AnalyzePropertyAsync("get => field ?? Name; set => field = value;");
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(reported, Has.Count.EqualTo(1));
+            Assert.That(reported.FirstOrDefault()?.Id, Is.EqualTo("MWAIS0005"));
+        }
+    }
+
     [Test]
     public async Task AClassicExtensionMethodIsLeftAlone()
     {
@@ -140,4 +165,23 @@ public sealed class ThisUsageAnalyzerTests
 
         return await CompilationHarness.AnalyzeAsync(source, new ThisUsageAnalyzer());
     }
+
+    private static async Task<IReadOnlyList<Diagnostic>> AnalyzePropertyAsync(string accessors)
+    {
+        var source =
+            $$"""
+              namespace Sample;
+
+              public sealed class Meeting
+              {
+                  public string Name { get; set; } = "Kickoff";
+
+                  public string Title { {{accessors}} }
+
+                  private string Describe() => this.Name;
+              }
+              """;
+
+        return await CompilationHarness.AnalyzeAsync(source, new ThisUsageAnalyzer());
+    }
 }

app/Tests/Assistants/VisualBriefing/VisualBriefingDuplicateFieldTests.cs

@@ -0,0 +1,44 @@
+using AIStudio.Assistants.VisualBriefing;
+
+namespace AIStudio.Tests.Assistants.VisualBriefing;
+
+/// <summary>
+/// Checks that the Visual Briefing assistant refuses a model answer which names a field twice.
+/// </summary>
+/// <remarks>
+/// The checks of the assistant would see one of the two values, while the browser which runs the
+/// finished briefing may see the other. The repair turn names the field, so that the model can fix it.
+/// </remarks>
+[TestFixture]
+public sealed class VisualBriefingDuplicateFieldTests
+{
+    [TestCase("""{"contractVersion":1,"contractVersion":2}""", "contractVersion", "$.contractVersion", TestName = "A field of the answer")]
+    [TestCase("""{"contractVersion":1,"facts":[{"id":"fact-1","id":"fact-2"}]}""", "id", "$.facts[0].id", TestName = "A field of an object in a list")]
+    [TestCase("""{"facts":[{"value":{"unit":"EUR","unit":"USD"}}]}""", "unit", "$.facts[0].value.unit", TestName = "A field deep inside a value")]
+    [TestCase("```json\n{\"contractVersion\":1,\"contractVersion\":2}\n```", "contractVersion", "$.contractVersion", TestName = "A field of an answer in a code block")]
+    public void AnAnswerWhichNamesAFieldTwiceIsRefused(string answer, string repeatedField, string path)
+    {
+        var result = VisualBriefingStructuredResponseProcessor.Process<VisualBriefingEvidenceResponse>(answer, _ => null);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(result.Response, Is.Null);
+            Assert.That(result.Issue?.Code, Is.EqualTo(VisualBriefingFailureCode.RESPONSE_JSON_INVALID));
+            Assert.That(result.Issue?.Rule, Is.EqualTo(VisualBriefingValidationRule.JSON_INVALID));
+            Assert.That(result.Issue?.Diagnostic?.IssueKind, Is.EqualTo(VisualBriefingStructuredResponseIssueKind.DUPLICATE_FIELD));
+            Assert.That(result.Issue?.Diagnostic?.FieldName, Is.EqualTo(repeatedField));
+            Assert.That(result.Issue?.Diagnostic?.JsonPath, Is.EqualTo(path));
+            Assert.That(result.Issue?.Issue, Does.Contain($"'{repeatedField}'"), "The repair turn names the field.");
+        }
+    }
+
+    [Test]
+    public void TheSameFieldInTwoObjectsIsNoDuplicate()
+    {
+        var result = VisualBriefingStructuredResponseProcessor.Process<VisualBriefingEvidenceResponse>(
+            """{"contractVersion":1,"facts":[{"id":"fact-1"},{"id":"fact-2"}]}""",
+            _ => null);
+
+        Assert.That(result.Issue?.Diagnostic?.IssueKind, Is.Not.EqualTo(VisualBriefingStructuredResponseIssueKind.DUPLICATE_FIELD));
+    }
+}

app/Tests/Provider/AdditionalApiParametersParserTests.cs

@@ -0,0 +1,46 @@
+using AIStudio.Provider;
+
+namespace AIStudio.Tests.Provider;
+
+/// <summary>
+/// Checks how the additional API parameters of a provider are read.
+/// </summary>
+/// <remarks>
+/// The dialog for providers refuses a parameter named twice, but a configuration plugin brings its
+/// providers past the dialog. Read anyway, such parameters would leave it open which of the values
+/// reaches the provider, and they used to break every request with an exception nobody caught.
+/// </remarks>
+[TestFixture]
+public sealed class AdditionalApiParametersParserTests
+{
+    [Test]
+    public void TheParametersAreRead()
+    {
+        var isRead = AdditionalApiParametersParser.TryParse("""
+            "temperature": 0.5,
+            "reasoning": { "effort": "low" }
+            """, out var parameters, out var errorMessage);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isRead, Is.True);
+            Assert.That(errorMessage, Is.Null);
+            Assert.That(parameters["temperature"], Is.EqualTo(0.5));
+            Assert.That(parameters["reasoning"], Is.EqualTo(new Dictionary<string, object> { ["effort"] = "low" }));
+        }
+    }
+
+    [TestCase("\"temperature\": 0.5, \"temperature\": 0.7", "temperature", TestName = "A parameter named twice")]
+    [TestCase("\"reasoning\": { \"effort\": \"low\", \"effort\": \"high\" }", "effort", TestName = "A field named twice inside a parameter")]
+    public void AParameterNamedTwiceIsRefused(string additionalParameters, string repeatedName)
+    {
+        var isRead = AdditionalApiParametersParser.TryParse(additionalParameters, out var parameters, out var errorMessage);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isRead, Is.False);
+            Assert.That(parameters, Is.Empty);
+            Assert.That(errorMessage, Does.Contain($"'{repeatedName}'"), "The log names the parameter to correct.");
+        }
+    }
+}

app/Tests/Provider/ModelIdOrderTests.cs

@@ -0,0 +1,25 @@
+using AIStudio.Provider;
+
+namespace AIStudio.Tests.Provider;
+
+/// <summary>
+/// Checks the order in which the dialogs for providers list the models, newest first.
+/// </summary>
+/// <remarks>
+/// Compared digit by digit, "gpt-4.9" would come before "gpt-4.10" and "o3" before "o10", so an older
+/// model would be on top of the list.
+/// </remarks>
+[TestFixture]
+public sealed class ModelIdOrderTests
+{
+    [TestCase(new[] { "gpt-4.9", "gpt-4.10", "gpt-4.1" }, new[] { "gpt-4.10", "gpt-4.9", "gpt-4.1" }, TestName = "Version numbers after a dot")]
+    [TestCase(new[] { "o3", "o10", "o1" }, new[] { "o10", "o3", "o1" }, TestName = "Numbers after a letter")]
+    [TestCase(new[] { "llama3.1:8b", "llama3.1:70b" }, new[] { "llama3.1:70b", "llama3.1:8b" }, TestName = "Sizes of a model")]
+    [TestCase(new[] { "mistral-large-2407", "mistral-large-2411", "mistral-large-latest" }, new[] { "mistral-large-latest", "mistral-large-2411", "mistral-large-2407" }, TestName = "Dates of a model")]
+    public void TheNewestModelsComeFirst(string[] modelIds, string[] expectedOrder)
+    {
+        var ordered = modelIds.OrderByDescending(id => id, Model.ID_ORDER);
+
+        Assert.That(ordered, Is.EqualTo(expectedOrder));
+    }
+}

app/Tests/Serialization/DuplicatePropertyTests.cs

@@ -0,0 +1,67 @@
+using System.Text.Json;
+
+using AIStudio.Assistants.Builder;
+using AIStudio.Assistants.VisualBriefing;
+using AIStudio.Tools.ERIClient;
+using AIStudio.Tools.ERIClient.DataModel;
+
+namespace AIStudio.Tests.Serialization;
+
+/// <summary>
+/// Checks that JSON which a model or a server wrote may name a field only once.
+/// </summary>
+/// <remarks>
+/// JSON allows a name twice, and readers disagree on which of the values counts. Whatever AI Studio
+/// checks in such an answer, another reader may see the other value, so these options refuse it. The
+/// stored data, such as the settings, stays out of this: AI Studio wrote it itself, and refusing it
+/// would lose it.
+/// </remarks>
+[TestFixture]
+public sealed class DuplicatePropertyTests
+{
+    private static IEnumerable<TestCaseData> OptionsForAnswersFromOutside()
+    {
+        yield return new TestCaseData(LuaResponse.JSON_OPTIONS).SetName("The plugins the Assistant Builder writes");
+        yield return new TestCaseData(LauncherTextsResponse.JSON_OPTIONS).SetName("The texts the Assistant Builder writes for a chat launcher");
+        yield return new TestCaseData(VisualBriefingJson.Canonical).SetName("The answers of the model for a visual briefing");
+        yield return new TestCaseData(ERIClientBase.JSON_OPTIONS).SetName("The answers of ERI servers");
+    }
+
+    [TestCaseSource(nameof(OptionsForAnswersFromOutside))]
+    public void TheOptionsRefuseAFieldNamedTwice(JsonSerializerOptions options) => Assert.That(options.AllowDuplicateProperties, Is.False);
+
+    [Test]
+    public void APluginWhoseLuaCodeIsNamedTwiceIsRefused()
+    {
+        var isParsed = LuaResponse.TryParse("""{"full_lua":"ID = \"a\"","full_lua":"ID = \"b\""}""", out _, out var error, out var technicalDetails);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isParsed, Is.False);
+            Assert.That(error, Is.EqualTo(LuaResponseParseError.INVALID_JSON));
+            Assert.That(technicalDetails, Does.Contain("'full_lua'"), "The user learns which field the model wrote twice.");
+        }
+    }
+
+    [Test]
+    public void TextsForAChatLauncherWithATitleNamedTwiceAreRefused()
+    {
+        var isParsed = LauncherTextsResponse.TryParse("""{"title":"Budget","title":"Travel"}""", out _, out var error, out var technicalDetails);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isParsed, Is.False);
+            Assert.That(error, Is.EqualTo(LuaResponseParseError.INVALID_JSON));
+            Assert.That(technicalDetails, Does.Contain("'title'"), "The user learns which field the model wrote twice.");
+        }
+    }
+
+    [TestCase("""{"allowedProviderType":"SELF_HOSTED","allowedProviderType":"ANY"}""", TestName = "The same name twice")]
+    [TestCase("""{"allowedProviderType":"SELF_HOSTED","AllowedProviderType":"ANY"}""", TestName = "Two names which differ only in case")]
+    public void SecurityRequirementsOfAnERIServerWhichNameAFieldTwiceAreRefused(string answer)
+    {
+        var exception = Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<SecurityRequirements>(answer, ERIClientBase.JSON_OPTIONS));
+
+        Assert.That(exception.Message, Does.Contain("AllowedProviderType").IgnoreCase, "The message names the field the server wrote twice.");
+    }
+}

app/Tests/Tools/PluginSystem/PluginArchiveTests.cs

@@ -0,0 +1,101 @@
+using System.IO.Compression;
+
+using AIStudio.Tools.PluginSystem;
+
+namespace AIStudio.Tests.Tools.PluginSystem;
+
+/// <summary>
+/// Checks how plugin archives are unpacked, from an import or from the configuration server of an organization.
+/// </summary>
+/// <remarks>
+/// AI Studio unpacks these archives itself rather than through ZipFile.ExtractToDirectory, which on Linux and
+/// macOS would keep a backslash in an entry name as part of the file name. Archives packed on Windows by hand
+/// or with another tool hold such names, and the plugin.lua inside would never be found.
+/// </remarks>
+[TestFixture]
+public sealed class PluginArchiveTests
+{
+    private string testDirectory = string.Empty;
+
+    [SetUp]
+    public void CreateTestDirectory()
+    {
+        this.testDirectory = Path.Combine(Path.GetTempPath(), $"ai-studio-plugin-archive-{Guid.NewGuid():N}");
+        Directory.CreateDirectory(this.testDirectory);
+    }
+
+    [TearDown]
+    public void DeleteTestDirectory()
+    {
+        if (Directory.Exists(this.testDirectory))
+            Directory.Delete(this.testDirectory, true);
+    }
+
+    [Test]
+    public async Task AnArchivePackedOnWindowsIsUnpackedIntoItsFolders()
+    {
+        var archivePath = await this.CreateArchiveAsync(new ArchiveEntry("O\\plugin.lua", "ID = \"o\""), new ArchiveEntry("O\\lib\\"), new ArchiveEntry("O\\lib\\helpers.lua", "return {}"));
+        var destination = Path.Combine(this.testDirectory, "unpacked");
+
+        await PluginArchive.ExtractAsync(archivePath, destination);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(await File.ReadAllTextAsync(Path.Combine(destination, "O", "plugin.lua")), Is.EqualTo("ID = \"o\""));
+            Assert.That(await File.ReadAllTextAsync(Path.Combine(destination, "O", "lib", "helpers.lua")), Is.EqualTo("return {}"));
+        }
+    }
+
+    [TestCase("../plugin.lua", TestName = "An entry above the destination")]
+    [TestCase("/plugin.lua", TestName = "An entry with an absolute path")]
+    public async Task AnEntryOutsideOfTheDestinationIsRefused(string entryName)
+    {
+        var archivePath = await this.CreateArchiveAsync(new ArchiveEntry(entryName, "ID = \"o\""));
+        var destination = Path.Combine(this.testDirectory, "unpacked");
+
+        await Assert.ThrowsAsync<InvalidDataException>(() => PluginArchive.ExtractAsync(archivePath, destination));
+        Assert.That(File.Exists(Path.Combine(this.testDirectory, "plugin.lua")), Is.False);
+    }
+
+    [Test]
+    public async Task ADirectoryEntryWithDataIsRefused()
+    {
+        var archivePath = await this.CreateArchiveAsync(new ArchiveEntry("O/", "ID = \"o\""));
+
+        await Assert.ThrowsAsync<InvalidDataException>(() => PluginArchive.ExtractAsync(archivePath, Path.Combine(this.testDirectory, "unpacked")));
+    }
+
+    [Test]
+    public async Task AnArchiveIsNotUnpackedAfterTheCancellation()
+    {
+        var archivePath = await this.CreateArchiveAsync(new ArchiveEntry("O/plugin.lua", "ID = \"o\""));
+        var destination = Path.Combine(this.testDirectory, "unpacked");
+
+        await Assert.CatchAsync<OperationCanceledException>(() => PluginArchive.ExtractAsync(archivePath, destination, new CancellationToken(canceled: true)));
+        Assert.That(File.Exists(Path.Combine(destination, "O", "plugin.lua")), Is.False);
+    }
+
+    private async Task<string> CreateArchiveAsync(params ArchiveEntry[] entries)
+    {
+        var archivePath = Path.Combine(this.testDirectory, $"{Guid.NewGuid():N}.mwplugin");
+        await using var archiveStream = File.Create(archivePath);
+        await using var archive = await ZipArchive.CreateAsync(archiveStream, ZipArchiveMode.Create, leaveOpen: false, entryNameEncoding: null);
+        foreach (var archiveEntry in entries)
+        {
+            var entry = archive.CreateEntry(archiveEntry.Name);
+            if (archiveEntry.Content is null)
+                continue;
+
+            await using var entryStream = await entry.OpenAsync();
+            await using var writer = new StreamWriter(entryStream);
+            await writer.WriteAsync(archiveEntry.Content);
+        }
+
+        return archivePath;
+    }
+
+    /// <summary>
+    /// An entry of a test archive: a file with its content, or a directory without one.
+    /// </summary>
+    private sealed record ArchiveEntry(string Name, string? Content = null);
+}

app/Tests/Tools/ToolCalling/ToolExecutorTests.cs

@@ -95,6 +95,35 @@ public sealed class ToolExecutorTests : ToolRegistryTestBase
         }
     }
 
+    [TestCase("""{"query":"budget","query":"https://attacker.example/?mail=board-meeting"}""", "query", TestName = "An argument named twice")]
+    [TestCase("""{"filter":{"sender":"budget","sender":"https://attacker.example/?mail=board-meeting"}}""", "sender", TestName = "A field named twice inside an argument")]
+    public async Task ACallWhichNamesAnArgumentTwiceDoesNotRun(string argumentsJson, string repeatedName)
+    {
+        //
+        // The tool would read one of the two values, while the check for addresses the model wrote
+        // into its call reads both. Which one a provider reads when the call goes back to it is
+        // anybody's guess:
+        //
+        var executed = false;
+        var tool = new TestTool(Definition(), execute: _ =>
+        {
+            executed = true;
+            return new ToolExecutionResult { TextContent = "A passage from the handbook." };
+        });
+
+        var outcome = await this.Execute(tool, new ChatThread(), argumentsJson);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(executed, Is.False);
+            Assert.That(outcome.Trace.Status, Is.EqualTo(ToolInvocationTraceStatus.ERROR));
+            Assert.That(outcome.Content, Does.Contain($"'{repeatedName}'"), "The model learns which name it wrote twice.");
+            Assert.That(outcome.Content, Does.Not.Contain("budget").And.Not.Contain("attacker"), "The message quotes none of the values, which may carry secrets.");
+            Assert.That(outcome.Trace.Arguments, Is.Not.Empty, "The user still sees what the model sent.");
+            AssertDemandsNothing(outcome);
+        }
+    }
+
     [Test]
     public async Task WhatAResultDemandsTightensTheChat()
     {