Align CTRF report writer overwrite behavior and add public IArtifactNamingService

src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.FileWriter.cs

@@ -7,72 +7,19 @@ namespace Microsoft.Testing.Extensions.CtrfReport;
 
 internal sealed partial class CtrfReportEngine
 {
-    private async Task<(string FileName, string? Warning)> WriteWithRetryAsync(string finalPath, byte[] bytes, bool fileNameExplicitlyProvided)
+    private async Task<(string FileName, string? Warning)> WriteAsync(string finalPath, byte[] bytes)
     {
-        // Explicit file names: use FileMode.Create (overwrite). Default-generated file
-        // names: use FileMode.CreateNew but retry with disambiguating suffixes when the
-        // file already exists, so concurrent runs (or two runs within the same second
-        // sharing the result directory) don't fail with IOException.
-        if (fileNameExplicitlyProvided)
-        {
-            bool willOverwrite = _fileSystem.ExistFile(finalPath);
-            await WriteBytesAsync(finalPath, FileMode.Create, bytes).ConfigureAwait(false);
-            return (
-                finalPath,
-                willOverwrite
-                    ? string.Format(CultureInfo.InvariantCulture, ExtensionResources.CtrfReportFileExistsAndWillBeOverwritten, finalPath)
-                    : null);
-        }
-
-        DateTimeOffset firstTry = _clock.UtcNow;
-        string directory = Path.GetDirectoryName(finalPath) ?? string.Empty;
-        string fileName = Path.GetFileName(finalPath);
-        SplitCtrfExtension(fileName, out string baseName, out string extension);
-        string candidate = finalPath;
-        int attempt = 0;
-
-        while (true)
-        {
-            _cancellationToken.ThrowIfCancellationRequested();
-
-            try
-            {
-                await WriteBytesAsync(candidate, FileMode.CreateNew, bytes).ConfigureAwait(false);
-                return (candidate, null);
-            }
-            catch (IOException) when (_fileSystem.ExistFile(candidate))
-            {
-                // The IOException was caused by the file already existing. Try a
-                // suffixed name. Any other IOException (disk full, permission, path
-                // too long, etc.) is not caught here and will propagate to the caller.
-                // Bound by both wall-clock (5s) and attempt count (1000) so we never
-                // spin forever in pathological cases like a clock that doesn't advance.
-                if (_clock.UtcNow - firstTry > TimeSpan.FromSeconds(5) || attempt >= 1_000)
-                {
-                    throw;
-                }
-
-                attempt++;
-                candidate = Path.Combine(directory, $"{baseName}_{attempt}{extension}");
-            }
-        }
-    }
-
-    // Split a file name into base + extension while preserving the CTRF
-    // double-extension convention (`*.ctrf.json`). The disambiguation suffix
-    // must land before `.ctrf.json` so that downstream CTRF readers continue to
-    // recognize the file by its conventional extension.
-    private static void SplitCtrfExtension(string fileName, out string baseName, out string extension)
-    {
-        const string ctrfJsonSuffix = ".ctrf.json";
-        if (fileName.EndsWith(ctrfJsonSuffix, StringComparison.OrdinalIgnoreCase) && fileName.Length > ctrfJsonSuffix.Length)
-        {
-            baseName = fileName.Substring(0, fileName.Length - ctrfJsonSuffix.Length);
-            extension = fileName.Substring(fileName.Length - ctrfJsonSuffix.Length);
-            return;
-        }
-
-        baseName = Path.GetFileNameWithoutExtension(fileName);
-        extension = Path.GetExtension(fileName);
+        // Always overwrite (FileMode.Create), regardless of whether the file name was explicitly
+        // provided or generated from the default
+        // <user>_<machine>_<asm>_<tfm>_<timestamp>.ctrf.json shape. Emit a warning when overwriting
+        // so users have a single, predictable rule to reason about — matching the TRX, HTML and
+        // JUnit report extensions.
+        bool willOverwrite = _fileSystem.ExistFile(finalPath);
+        await WriteBytesAsync(finalPath, FileMode.Create, bytes).ConfigureAwait(false);
+        return (
+            finalPath,
+            willOverwrite
+                ? string.Format(CultureInfo.InvariantCulture, ExtensionResources.CtrfReportFileExistsAndWillBeOverwritten, finalPath)
+                : null);
     }
 }

src/Platform/Microsoft.Testing.Extensions.CtrfReport/CtrfReportEngine.cs

@@ -19,12 +19,12 @@ public CtrfReportEngine(ReportEngineContext context)
 
     private async Task<(string FileName, string? Warning)> GenerateReportCoreAsync(CapturedTestResult[] results, DateTimeOffset finishTime)
     {
-        (string finalPath, bool fileNameExplicitlyProvided) = ResolveOutputPath(
+        (string finalPath, _) = ResolveOutputPath(
             CtrfReportGeneratorCommandLine.CtrfReportFileNameOptionName,
             () => BuildDefaultFileName(finishTime));
 
         byte[] bytes = BuildCtrfJson(results, finishTime);
 
-        return await WriteWithRetryAsync(finalPath, bytes, fileNameExplicitlyProvided).ConfigureAwait(false);
+        return await WriteAsync(finalPath, bytes).ConfigureAwait(false);
     }
 }

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs

@@ -64,7 +64,7 @@ private async Task<BuildContext> SetupCommonServicesAsync(
         serviceProvider.TryAddService(_testApplicationModuleInfo);
         serviceProvider.TryAddService(testHostControllerInfo);
         serviceProvider.TryAddService(systemClock);
-
+        serviceProvider.TryAddService(new ArtifactNamingService(_testApplicationModuleInfo, systemEnvironment, systemClock));
         SystemMonitor systemMonitor = new();
         serviceProvider.TryAddService(systemMonitor);
         SystemMonitorAsyncFactory systemMonitorAsyncFactory = new();

src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt

@@ -1 +1,6 @@
 #nullable enable
+Microsoft.Testing.Platform.Services.ArtifactFileNameSanitizer
+Microsoft.Testing.Platform.Services.ArtifactNamingService
+Microsoft.Testing.Platform.Services.ArtifactNamingService.ArtifactNamingService(Microsoft.Testing.Platform.Services.ITestApplicationModuleInfo! testApplicationModuleInfo, Microsoft.Testing.Platform.Helpers.IEnvironment! environment, Microsoft.Testing.Platform.Helpers.IClock! clock) -> void
+Microsoft.Testing.Platform.Services.ArtifactNamingService.ResolveFileName(string! template) -> string!
+static Microsoft.Testing.Platform.Services.ArtifactFileNameSanitizer.ReplaceInvalidFileNameChars(string! fileName) -> string!

src/Platform/Microsoft.Testing.Platform/PublicAPI/PublicAPI.Unshipped.txt

@@ -1 +1,4 @@
 #nullable enable
+Microsoft.Testing.Platform.Services.IArtifactNamingService
+Microsoft.Testing.Platform.Services.IArtifactNamingService.ResolveFileName(string! template) -> string!
+static Microsoft.Testing.Platform.Services.ServiceProviderExtensions.GetArtifactNamingService(this System.IServiceProvider! serviceProvider) -> Microsoft.Testing.Platform.Services.IArtifactNamingService!

src/Platform/Microsoft.Testing.Platform/Services/ArtifactFileNameSanitizer.cs

@@ -0,0 +1,55 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.Testing.Platform.Helpers;
+
+namespace Microsoft.Testing.Platform.Services;
+
+// Mirrors the extension-side Microsoft.Testing.Extensions.ReportFileNameSanitizer (linked source in
+// src/Platform/SharedExtensionHelpers/ReportFileNameSanitizer.cs). The extension copy has SHIPPED
+// InternalAPI entries in five extensions, so it cannot be moved without churn/risk. This core-internal
+// copy exists pending future consolidation of the two.
+internal static partial class ArtifactFileNameSanitizer
+{
+    private const char ReplacementChar = '_';
+    private static readonly Regex ReservedFileNamesRegex = BuildReservedFileNameRegex();
+
+    internal static string ReplaceInvalidFileNameChars(string fileName)
+    {
+        char[] result = new char[fileName.Length];
+
+        for (int i = 0; i < fileName.Length; ++i)
+        {
+            result[i] = IsInvalidFileNameChar(fileName[i]) ? ReplacementChar : fileName[i];
+        }
+
+        string replaced = new string(result).TrimEnd();
+        ArgumentGuard.Ensure(replaced.Length > 0, nameof(fileName), $"File name {fileName} is empty after sanitizing invalid characters.");
+
+        if (IsReservedFileName(replaced))
+        {
+            replaced = ReplacementChar + replaced; // Cannot add to the end because it can have extensions.
+        }
+
+        return replaced;
+    }
+
+    private static bool IsInvalidFileNameChar(char c)
+        => c is < ' ' or '"' or '<' or '>' or '|' or ':' or '*' or '?' or '\\' or '/' or '@' or '(' or ')' or '^' or ' ';
+
+    private static bool IsReservedFileName(string fileName) =>
+        // CreateFile:
+        // The following reserved device names cannot be used as the name of a file:
+        // CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9,
+        // LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, and LPT9.
+        // Also avoid these names followed by an extension, for example, NUL.tx7.
+        // Windows NT: CLOCK$ is also a reserved device name.
+        ReservedFileNamesRegex.IsMatch(fileName);
+
+#if NET7_0_OR_GREATER
+    [GeneratedRegex(@"(?i:^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9]|CLOCK\$)(\..*)?)$", RegexOptions.None, "en-150")]
+    private static partial Regex BuildReservedFileNameRegex();
+#else
+    private static Regex BuildReservedFileNameRegex() => new(@"(?i:^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9]|CLOCK\$)(\..*)?)$");
+#endif
+}

src/Platform/Microsoft.Testing.Platform/Services/ArtifactNamingService.cs

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.Testing.Platform.Helpers;
+
+namespace Microsoft.Testing.Platform.Services;
+
+internal sealed class ArtifactNamingService : IArtifactNamingService
+{
+    private readonly ITestApplicationModuleInfo _testApplicationModuleInfo;
+    private readonly IEnvironment _environment;
+    private readonly IClock _clock;
+
+    public ArtifactNamingService(ITestApplicationModuleInfo testApplicationModuleInfo, IEnvironment environment, IClock clock)
+    {
+        _testApplicationModuleInfo = testApplicationModuleInfo;
+        _environment = environment;
+        _clock = clock;
+    }
+
+    public string ResolveFileName(string template)
+    {
+        string processName = Path.GetFileNameWithoutExtension(_testApplicationModuleInfo.GetCurrentTestApplicationFullPath());
+        string processId = _environment.ProcessId.ToString(CultureInfo.InvariantCulture);
+        Dictionary<string, string> replacements = ArtifactNamingHelper.GetStandardReplacements(processName, processId, _clock.UtcNow);
+        string resolved = ArtifactNamingHelper.ResolveTemplate(template, replacements);
+
+        string directoryPart = Path.GetDirectoryName(resolved) ?? string.Empty;
+        string sanitized = ArtifactFileNameSanitizer.ReplaceInvalidFileNameChars(Path.GetFileName(resolved));
+
+        return directoryPart.Length == 0 ? sanitized : Path.Combine(directoryPart, sanitized);
+    }
+}

src/Platform/Microsoft.Testing.Platform/Services/IArtifactNamingService.cs

@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Microsoft.Testing.Platform.Services;
+
+/// <summary>
+/// Provides file-name templating for artifacts produced by the test platform and its extensions.
+/// </summary>
+/// <remarks>
+/// The service expands the standard placeholders and sanitizes the resulting leaf file name so it is
+/// valid on the current operating system. It is registered as a common service and can be resolved
+/// through <see cref="ServiceProviderExtensions.GetArtifactNamingService(System.IServiceProvider)"/>.
+/// </remarks>
+public interface IArtifactNamingService
+{
+    /// <summary>
+    /// Resolves a file-name template into a concrete file name.
+    /// </summary>
+    /// <param name="template">
+    /// The template to resolve. The following placeholders are expanded:
+    /// <list type="bullet">
+    /// <item><description><c>{pname}</c> — the current test application process name.</description></item>
+    /// <item><description><c>{pid}</c> — the current process id.</description></item>
+    /// <item><description><c>{asm}</c> — the entry assembly name.</description></item>
+    /// <item><description><c>{tfm}</c> — the short target framework moniker (including platform).</description></item>
+    /// <item><description><c>{arch}</c> — the process architecture.</description></item>
+    /// <item><description><c>{time}</c> — the current UTC timestamp.</description></item>
+    /// </list>
+    /// Unknown placeholders are preserved as-is. Any directory portion of the template is preserved
+    /// while only the leaf file name is sanitized.
+    /// </param>
+    /// <returns>The resolved file name, with the leaf sanitized to be valid for the current file system.</returns>
+    string ResolveFileName(string template);
+}

src/Platform/Microsoft.Testing.Platform/Services/ServiceProviderExtensions.cs

@@ -71,6 +71,14 @@ public static IMessageBus GetMessageBus(this IServiceProvider serviceProvider)
     public static IConfiguration GetConfiguration(this IServiceProvider serviceProvider)
         => serviceProvider.GetRequiredService<IConfiguration>();
 
+    /// <summary>
+    /// Gets the artifact naming service from the <see cref="IServiceProvider"/>.
+    /// </summary>
+    /// <param name="serviceProvider">The service provider.</param>
+    /// <returns>The artifact naming service.</returns>
+    public static IArtifactNamingService GetArtifactNamingService(this IServiceProvider serviceProvider)
+        => serviceProvider.GetRequiredService<IArtifactNamingService>();
+
     /// <summary>
     /// Gets the command line options from the <see cref="IServiceProvider"/>.
     /// </summary>

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs

@@ -393,7 +393,7 @@ public async Task GenerateReportAsync_DefaultFileName_IncludesModuleNameAndTarge
     {
         string? pathSeen = null;
         _ = _fileSystem.Setup(x => x.ExistFile(It.IsAny<string>())).Returns(false);
-        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.CreateNew))
+        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.Create))
             .Returns<string, FileMode>((path, _) =>
             {
                 pathSeen = path;
@@ -485,20 +485,19 @@ public async Task GenerateReportAsync_ExplicitAbsolutePath_OverridesResultsDirec
     }
 
     [TestMethod]
-    public async Task GenerateReportAsync_AppendsDisambiguatingSuffix_When_DefaultFileExists()
+    public async Task GenerateReportAsync_OverwritesAndWarns_When_DefaultFileExists()
     {
-        var bytesSeen = new List<string>();
-        int callCount = 0;
-        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.CreateNew))
+        // Default-name path uses the same overwrite-and-warn semantics as the explicit-name
+        // path: a single, predictable rule. When the file already exists, the engine
+        // overwrites it (FileMode.Create) and surfaces the CtrfReportFileExistsAndWillBeOverwritten
+        // warning.
+        string? pathSeen = null;
+        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.Create))
             .Returns<string, FileMode>((path, _) =>
             {
-                callCount++;
-                bytesSeen.Add(path);
-                return callCount == 1
-                    ? throw new IOException("file exists")
-                    : new MemoryFileStream();
+                pathSeen = path;
+                return new MemoryFileStream();
             });
-
         _ = _fileSystem.Setup(x => x.ExistFile(It.IsAny<string>())).Returns(true);
 
         _ = _configurationMock.SetupGet(_ => _[It.IsAny<string>()]).Returns(string.Empty);
@@ -522,18 +521,22 @@ public async Task GenerateReportAsync_AppendsDisambiguatingSuffix_When_DefaultFi
             0,
             CancellationToken.None));
 
-        (string finalPath, _) = await engine.GenerateReportAsync([Captured("a", "A", "passed")]);
+        (string finalPath, string? warning) = await engine.GenerateReportAsync([Captured("a", "A", "passed")]);
 
-        Assert.AreEqual(2, callCount);
-        Assert.AreEqual(bytesSeen[1], finalPath);
-        Assert.Contains("_1.ctrf.json", finalPath);
+        Assert.AreEqual(pathSeen, finalPath);
+        Assert.DoesNotContain("_1.ctrf.json", finalPath);
+        Assert.IsNotNull(warning);
+        Assert.Contains(finalPath, warning!);
     }
 
     [TestMethod]
-    public async Task GenerateReportAsync_PropagatesIOException_When_FileDoesNotExist()
+    public async Task GenerateReportAsync_PropagatesIOException_When_WriteFails()
     {
+        // An IOException during the write (e.g. disk full, permission denied, path too
+        // long) must propagate to the caller — there is no longer any disambiguation
+        // loop that could mask such failures behind a retry budget.
         int callCount = 0;
-        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.CreateNew))
+        _ = _fileSystem.Setup(x => x.NewFileStream(It.IsAny<string>(), FileMode.Create))
             .Returns<string, FileMode>((path, _) =>
             {
                 callCount++;