Address second round of PR review feedback

- Remove de-duplication of MTPProjects: the old CreateXUnitV3WorkItems
  task didn't deduplicate either; defer to MSBuild item semantics and
  keep the task minimal.
- Drop the TrxReportFilename defensive validation: MTP itself rejects
  values containing path separators, so the redundant pre-check just
  added complexity. Filename is still quoted in the emitted command so
  spaces are safe.
- Simplify the timeout-parsing block to use 'out timeout' directly with
  a default reset on failure (TimeSpan.TryParse zeroes the out var when
  it fails).
- Shrink the MTPTrxReportFilename property documentation in the
  targets file to a one-liner.
- Update tests: replace InvalidTrxReportFilenameIsRejected with
  TrxReportFilenameIsPassedThroughVerbatim and replace
  DuplicatesByIdentityAndAdditionalPropertiesAreCollapsed with
  DuplicateInputsArePassedThroughAsSeparateWorkItems.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Evangelink

src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/CreateMTPWorkItemsTests.cs

@@ -89,14 +89,17 @@ public void CustomTrxReportFilenameIsQuoted()
         [InlineData("sub\\results.trx")]
         [InlineData("with\"quote.trx")]
         [InlineData("")]
-        public void InvalidTrxReportFilenameIsRejected(string filename)
+        public void TrxReportFilenameIsPassedThroughVerbatim(string filename)
         {
+            // The task no longer validates the filename - MTP itself rejects values containing
+            // path separators, and the value is quoted in the command so spaces are safe.
             var task = CreateTask();
             task.TrxReportFilename = filename;
             task.MTPProjects = new[] { CreateProject("MyApp.Tests.csproj") };
 
-            task.Execute().Should().BeFalse();
-            task.MTPWorkItems.Should().BeEmpty();
+            task.Execute().Should().BeTrue();
+            task.MTPWorkItems.Single().GetMetadata("Command")
+                .Should().Contain($"--report-trx-filename \"{filename}\"");
         }
 
         [Fact]
@@ -144,8 +147,11 @@ public void InvalidTimeoutFallsBackToDefaultWithWarning()
         }
 
         [Fact]
-        public void DuplicatesByIdentityAndAdditionalPropertiesAreCollapsed()
+        public void DuplicateInputsArePassedThroughAsSeparateWorkItems()
         {
+            // The task does not deduplicate - it mirrors the old CreateXUnitV3WorkItems behavior
+            // and trusts the caller to provide a clean item list (MSBuild's RemoveDuplicates can
+            // be used upstream if needed).
             var task = CreateTask();
             task.MTPProjects = new[]
             {
@@ -154,7 +160,7 @@ public void DuplicatesByIdentityAndAdditionalPropertiesAreCollapsed()
             };
 
             task.Execute().Should().BeTrue();
-            task.MTPWorkItems.Should().HaveCount(1);
+            task.MTPWorkItems.Should().HaveCount(2);
         }
 
         [Fact]

src/Microsoft.DotNet.Helix/Sdk/CreateMTPWorkItems.cs

@@ -75,16 +75,7 @@ public override bool Execute()
 
         private async Task ExecuteAsync()
         {
-            // De-duplicate inputs by (Identity, AdditionalProperties). Protects against the
-            // same project being included by overlapping globs while preserving legitimate
-            // same-Identity entries that differ in AdditionalProperties (the canonical
-            // multi-TFM pattern).
-            var deduped = MTPProjects
-                .GroupBy(p => (p.ItemSpec, p.GetMetadata("AdditionalProperties") ?? string.Empty))
-                .Select(g => g.First())
-                .ToArray();
-
-            MTPWorkItems = (await Task.WhenAll(deduped.Select(PrepareWorkItem))).Where(wi => wi != null).ToArray();
+            MTPWorkItems = (await Task.WhenAll(MTPProjects.Select(PrepareWorkItem))).Where(wi => wi != null).ToArray();
         }
 
         /// <summary>
@@ -113,39 +104,13 @@ private async Task<ITaskItem> PrepareWorkItem(ITaskItem mtpProject)
                 assemblyBaseName = assemblyBaseName.Substring(0, assemblyBaseName.Length - 4);
             }
 
-            // Defense in depth: reject values that would let the TRX file escape the work
-            // item working directory or break shell tokenization in surprising ways. Path
-            // separators in particular would cause '--results-directory .' + the filename
-            // to land outside the cwd where neither the arcade Python reporter nor the
-            // Helix job monitor scan for results.
-            if (string.IsNullOrEmpty(TrxReportFilename) ||
-                TrxReportFilename.IndexOfAny(new[] { '/', '\\', '"' }) >= 0)
-            {
-                Log.LogError($"Invalid value \"{TrxReportFilename}\" provided for TrxReportFilename; it must be a non-empty filename with no path separators or quote characters.");
-                return null;
-            }
-
             // MTP test apps are self-hosting executables. Run the assembly directly with
             // 'dotnet exec'. The reporter args below require the test project to reference
             // Microsoft.Testing.Extensions.TrxReport. MSTest.Sdk references it transitively;
             // xUnit v3 projects built with Microsoft.DotNet.Arcade.Sdk's XUnitV3 targets get
             // it implicitly as well. Other MTP-based frameworks must add the package.
-            //
-            //   --results-directory .  -> TRX is written next to the assembly, which is the
-            //                             work item's cwd and is scanned by the arcade Python
-            //                             reporter / EnableHelixJobMonitor.
-            //   --report-trx           -> enable the TrxReport extension.
-            //   --report-trx-filename  -> deterministic filename so the parser finds it. The
-            //                             value is wrapped in double quotes so spaces or
-            //                             other shell-significant characters in the filename
-            //                             do not split the argument.
-            //
-            // Note about the AzureDevOps reporter (Microsoft.Testing.Extensions.AzureDevOpsReport):
-            // it activates only when '--report-azdo' is explicitly passed AND TF_BUILD=true.
-            // We never pass '--report-azdo' here, so there is no risk of a duplicate Azure
-            // DevOps test run on top of the one the Helix Sdk already opened. If a user has
-            // wired '--report-azdo' into their test project they should remove it for Helix
-            // runs (it conflicts with the Helix-managed run).
+            // The filename is wrapped in double quotes so spaces in user-provided values do
+            // not split the argument. MTP itself rejects values containing path separators.
             string reporterArgs =
                 $"--results-directory . --report-trx --report-trx-filename \"{TrxReportFilename}\"";
 
@@ -158,16 +123,10 @@ private async Task<ITaskItem> PrepareWorkItem(ITaskItem mtpProject)
             Log.LogMessage($"Creating MTP work item with properties Identity: {assemblyName}, PayloadDirectory: {publishDirectory}, Command: {command}");
 
             TimeSpan timeout = TimeSpan.FromMinutes(5);
-            if (!string.IsNullOrEmpty(MTPWorkItemTimeout))
+            if (!string.IsNullOrEmpty(MTPWorkItemTimeout) && !TimeSpan.TryParse(MTPWorkItemTimeout, out timeout))
             {
-                if (!TimeSpan.TryParse(MTPWorkItemTimeout, out TimeSpan parsedTimeout))
-                {
-                    Log.LogWarning($"Invalid value \"{MTPWorkItemTimeout}\" provided for MTPWorkItemTimeout; falling back to default value of \"00:05:00\" (5 minutes)");
-                }
-                else
-                {
-                    timeout = parsedTimeout;
-                }
+                Log.LogWarning($"Invalid value \"{MTPWorkItemTimeout}\" provided for MTPWorkItemTimeout; falling back to default value of \"00:05:00\" (5 minutes)");
+                timeout = TimeSpan.FromMinutes(5);
             }
 
             var result = new Microsoft.Build.Utilities.TaskItem(assemblyName, new Dictionary<string, string>()

src/Microsoft.DotNet.Helix/Sdk/tools/mtp-runner/MTPRunner.targets

@@ -19,12 +19,7 @@
 
   <PropertyGroup>
     <_MTPPublishTargetsPath>$(MSBuildThisFileDirectory)../xunit-runner/XUnitPublish.targets</_MTPPublishTargetsPath>
-    <!--
-      Name of the TRX file produced by Microsoft.Testing.Extensions.TrxReport. The arcade
-      Python TRXFormat parser (and EnableHelixJobMonitor's *.trx sweep) will pick up any
-      *.trx in the work item working directory, so the exact name only matters if the user
-      has multiple consumers expecting a specific filename.
-    -->
+    <!-- Name of the TRX file produced by Microsoft.Testing.Extensions.TrxReport. -->
     <MTPTrxReportFilename Condition="'$(MTPTrxReportFilename)' == ''">testResults.trx</MTPTrxReportFilename>
   </PropertyGroup>