Exit 1 for an existing output folder that can't be written to [patch]

CLAUDE.md

@@ -149,9 +149,11 @@ help text and exited 0.
 
 `Run` is a one-line `Environment.Exit` wrapper around `Execute(Arguments)`, which returns the exit code so a
 whole run is testable. `Execute` also turns an `IOException` or `UnauthorizedAccessException` from
-`ProcessDirectory` into exit 1. Only the up-front `Directory.CreateDirectory` and `Directory.GetFiles`
-can raise one, since each file's own failures are caught inside the batch, and a folder that cannot be
-created or listed used to end the run with a stack trace and exit 134.
+`ProcessDirectory` into exit 1. Only the up-front `Directory.CreateDirectory`, the `EnsureWritable` probe
+and `Directory.GetFiles` can raise one, since each file's own failures are caught inside the batch, and
+a folder that cannot be created or listed used to end the run with a stack trace and exit 134. The probe
+writes and deletes a dot-named temp file, because `CreateDirectory` is a no-op on an existing folder:
+without it a read-only output folder failed every icon one by one and exited 2.
 
 ### Known Rough Edges
 

IconHelper.Test/ProcessDirectoryTests.cs

@@ -228,6 +228,75 @@ public void AnOutputFolderUnderAFileExitsWithInvalidArgumentsInsteadOfThrowing()
 		Assert.AreEqual(IconHelper.ExitInvalidArguments, IconHelper.Execute(ArgumentsFor(input, output)));
 	}
 
+	[TestMethod]
+	public void AnExistingReadOnlyOutputFolderExitsWithInvalidArgumentsAndWritesNothing()
+	{
+		// Mode bits are POSIX; Windows read-only folders are an attribute Explorer itself ignores.
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Read-only folder modes are POSIX only.");
+			return;
+		}
+
+		using TempDirectory temp = new();
+		string input = temp.Combine("in");
+		string output = temp.Combine("out");
+		Directory.CreateDirectory(input);
+		Directory.CreateDirectory(output);
+		WritePng(Path.Join(input, "a.png"), 64);
+		WritePng(Path.Join(input, "b.png"), 64);
+
+		UnixFileMode original = File.GetUnixFileMode(output);
+		File.SetUnixFileMode(output, UnixFileMode.UserRead | UnixFileMode.UserExecute | UnixFileMode.GroupRead | UnixFileMode.GroupExecute | UnixFileMode.OtherRead | UnixFileMode.OtherExecute);
+		try
+		{
+			if (CanCreateAFileIn(output))
+			{
+				// Root, and some container file systems, ignore mode bits, so there is nothing to test.
+				Assert.Inconclusive("This process can write to a mode 555 folder, so the folder is not read-only to it.");
+			}
+
+			Assert.AreEqual(IconHelper.ExitInvalidArguments, IconHelper.Execute(ArgumentsFor(input, output)));
+			Assert.IsEmpty(Directory.GetFileSystemEntries(output), "Nothing should be written, the write probe included.");
+		}
+		finally
+		{
+			File.SetUnixFileMode(output, original);
+		}
+	}
+
+	[TestMethod]
+	public void LeavesNoWriteProbeBehindInAWritableOutputFolder()
+	{
+		using TempDirectory temp = new();
+		string input = temp.Combine("in");
+		string output = temp.Combine("out");
+		Directory.CreateDirectory(input);
+		WritePng(Path.Join(input, "icon.png"), 64);
+
+		Assert.AreEqual(IconHelper.ExitSuccess, IconHelper.Execute(ArgumentsFor(input, output)));
+		CollectionAssert.AreEqual(new[] { Path.Join(output, "icon.png") }, Directory.GetFileSystemEntries(output));
+	}
+
+	private static bool CanCreateAFileIn(string directory)
+	{
+		string probe = Path.Join(directory, $"probe-{Guid.NewGuid():N}");
+		try
+		{
+			File.WriteAllBytes(probe, []);
+			File.Delete(probe);
+			return true;
+		}
+		catch (UnauthorizedAccessException)
+		{
+			return false;
+		}
+		catch (IOException)
+		{
+			return false;
+		}
+	}
+
 	[TestMethod]
 	public void ExecuteReportsInvalidArgumentsForAMissingInput()
 	{

IconHelper/IconHelper.cs

@@ -89,7 +89,7 @@ internal static int Execute(Arguments args)
 		catch (Exception exception) when (exception is IOException or UnauthorizedAccessException)
 		{
 			// Validate can only check that the paths are well formed. Whether the output folder can be
-			// created, or the input folder listed, is known only on trying, and both happen before the
+			// created and written to, or the input folder listed, is known only on trying, and both happen before the
 			// per-file catch, so without this an unusable folder ended the run with a stack trace and
 			// exit 134. Each file's own failures never reach here: ProcessDirectory catches those.
 			Console.WriteLine($"Argument validation failed:\n\t{exception.Message}");
@@ -128,6 +128,7 @@ internal static BatchResult ProcessDirectory(Arguments args, Color color)
 		}
 
 		Directory.CreateDirectory(outputDirectory);
+		EnsureWritable(outputDirectory);
 
 		int processed = 0;
 		int failed = 0;
@@ -188,6 +189,20 @@ internal static BatchResult ProcessDirectory(Arguments args, Color color)
 		return new BatchResult(processed, failed);
 	}
 
+	/// <summary>
+	/// Throws the <see cref="IOException"/> or <see cref="UnauthorizedAccessException"/> that writing
+	/// an icon would, if the output folder cannot take a new file. CreateDirectory is a no-op on a
+	/// folder that already exists, so on its own it let a read-only one through, and every icon was
+	/// then decoded and processed only to fail at the save, exiting 2 as if the inputs were bad. The
+	/// probe is dot-named, so even a copy left behind by a crash is skipped as metadata.
+	/// </summary>
+	private static void EnsureWritable(AbsoluteDirectoryPath outputDirectory)
+	{
+		FileName probeName = FileName.Create<FileName>($".iconhelper-write-probe-{Guid.NewGuid():N}");
+		AbsoluteFilePath probePath = outputDirectory / probeName;
+		using FileStream probe = new(probePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.DeleteOnClose);
+	}
+
 	/// <summary>
 	/// Names that Finder and Explorer drop into any folder they open: dot-files such as .DS_Store and
 	/// ._ AppleDouble files, plus Thumbs.db, ehthumbs.db and desktop.ini. They are not icons, so they

README.md

@@ -284,7 +284,7 @@ already contains its own output will not reprocess those files.
 | Code | Meaning |
 |------|---------|
 | `0` | Every file was processed successfully. Also returned for `--help` and `--version` |
-| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an unrecognised `--color`, or `padding * 2 >= size` |
+| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an `--output` directory that cannot be created or written to, an unrecognised `--color`, or `padding * 2 >= size` |
 | `2` | The batch ran to completion but at least one file could not be processed |
 
 Code `2` means the run finished and the remaining icons were still written. Check the summary line