Fix RepositoryPaths.BuildVariables temp file deletion issue

[Copilot]

The RepositoryPaths.BuildVariables property was failing with a TypeInitializationException when the temporary build variables file gets deleted by Windows cleanup processes, making the entire RepositoryPaths class unusable.

Problem

System.TypeInitializationException : The type initializer for 'IntelliTect.Multitool.RepositoryPaths' threw an exception.
System.IO.FileNotFoundException : Could not find file 'C:\Users\...\IntelliTect.MultiTool.BuildVariables.tmp'.

This exception was originally thrown at this call stack:
    Microsoft.Win32.SafeHandles.SafeFileHandle.CreateFile(string, System.IO.FileMode, System.IO.FileAccess, System.IO.FileShare, System.IO.FileOptions)
    Microsoft.Win32.SafeHandles.SafeFileHandle.Open(string, System.IO.FileMode, System.IO.FileAccess, System.IO.FileShare, System.IO.FileOptions, long, System.IO.UnixFileMode?)
    System.IO.Strategies.OSFileStreamStrategy.OSFileStreamStrategy(string, System.IO.FileMode, System.IO.FileAccess, System.IO.FileShare, System.IO.FileOptions, long, System.IO.UnixFileMode?)
    System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(string, System.IO.FileMode, System.IO.FileAccess, System.IO.FileShare, System.IO.FileOptions, long, System.IO.UnixFileMode?)
    System.IO.StreamReader.ValidateArgsAndOpenPath(string, System.Text.Encoding, int)
    System.IO.File.ReadAllLines(string, System.Text.Encoding)
    IntelliTect.Multitool.RepositoryPaths.RepositoryPaths()

This occurred when:

• Project sat idle for a week or two without any work
• Windows cleanup processes purged the temp file
• Opening the project and running tests would fail immediately

Root Cause

The BuildVariables property was initialized eagerly as a static readonly property that immediately tried to read from a temp file without any error handling:

public static ReadOnlyDictionary<string, string?> BuildVariables { get; } = new(File.ReadAllLines(Path.Combine(Path.GetTempPath(), BuildVariableFileName))
    .Select(line => line.Split("::"))
    .ToDictionary(split => split[0].Trim(),
    split => !string.IsNullOrEmpty(split[1]) ? split[1].Trim() : null));

Solution

Changed BuildVariables to use lazy initialization with proper error handling:

1. Lazy Loading: Property now initializes only when first accessed
2. Error Handling: Catches FileNotFoundException and DirectoryNotFoundException
3. Graceful Degradation: Returns empty dictionary when temp file doesn't exist
4. Backward Compatibility: All existing code uses TryGetValue() which handles missing keys appropriately

Changes Made

• RepositoryPaths.cs: Replaced eager initialization with lazy loading and error handling
• RepositoryPaths.Tests.cs: Added test BuildVariables_HandlesFileNotFound_Gracefully to verify the fix

Verification

• ✅ No more TypeInitializationException when temp file is missing
• ✅ BuildVariables returns empty dictionary gracefully when file doesn't exist
• ✅ All existing functionality works when temp file is present
• ✅ GetDefaultRepoRoot() handles missing BuildVariables appropriately
• ✅ All existing tests continue to pass

The fix is minimal and surgical, maintaining full backward compatibility while resolving the crash scenario.

Fixes #137.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull Request Overview

This PR refactors BuildVariables to defer loading until first access and adds error handling for missing temp files or directories, preventing the static initializer crash. It also introduces a unit test to confirm graceful degradation when the temp file is deleted.

• Switch BuildVariables from eager static initialization to a lazily-loaded getter with LoadBuildVariables().
• Catch FileNotFoundException and DirectoryNotFoundException, returning an empty dictionary.
• Add BuildVariables_HandlesFileNotFound_Gracefully test to verify behavior when the temp file is absent.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
IntelliTect.Multitool/RepositoryPaths.cs	Replaced eager initialization with lazy loading and error handling.
IntelliTect.Multitool.Tests/RepositoryPaths.Tests.cs	Added test for missing temp file scenario and reset cache via reflection.

Comments suppressed due to low confidence (1)

IntelliTect.Multitool.Tests/RepositoryPaths.Tests.cs:47

• [nitpick] Consider adding a test for the DirectoryNotFoundException case to cover the branch when the temp directory itself is missing.

    [Fact]

[Copilot]

The lazy initialization of _buildVariables isn’t thread-safe. Consider using Lazy or a lock to prevent race conditions if BuildVariables is accessed concurrently.

Suggested change

	public static ReadOnlyDictionary<string, string?> BuildVariables
	{
	get
	{
	if (_buildVariables == null)
	{
	_buildVariables = LoadBuildVariables();
	}
	return _buildVariables;
	}
	}
	private static ReadOnlyDictionary<string, string?>? _buildVariables;
	public static ReadOnlyDictionary<string, string?> BuildVariables => _buildVariables.Value;
	private static readonly Lazy<ReadOnlyDictionary<string, string?>> _buildVariables =
	new Lazy<ReadOnlyDictionary<string, string?>>(LoadBuildVariables);

[Copilot]

Using Split("::") splits on individual ':' characters rather than the literal "::" token. If the intent is to split on "::", use Split(new[] { "::" }, StringSplitOptions.None) for correct behavior.

Suggested change

	.Select(line => line.Split("::"))
	.Select(line => line.Split(new[] { "::" }, StringSplitOptions.None))

[Copilot]

[nitpick] Allocating a new empty dictionary on each catch can be wasteful. Consider defining a static readonly empty ReadOnlyDictionary to reuse and avoid repeated allocations.

Suggested change

	return new ReadOnlyDictionary<string, string?>(new Dictionary<string, string?>());
	}
	catch (DirectoryNotFoundException)
	{
	// Return empty dictionary if the temp directory doesn't exist
	return new ReadOnlyDictionary<string, string?>(new Dictionary<string, string?>());
	return _emptyReadOnlyDictionary;
	}
	catch (DirectoryNotFoundException)
	{
	// Return empty dictionary if the temp directory doesn't exist
	return _emptyReadOnlyDictionary;

[Copilot]

[nitpick] Using reflection to reset a private static field makes the test brittle. Consider adding an internal reset method or using a public test hook to clear the cache more robustly.