Fix: Resolve correct pwsh path if installed via Windows Store

[BornToBeRoot]

Changes proposed in this pull request

• Resolve correct pwsh path if installed via Windows Store to apply theme

Related issue(s)

• Fixes PowerShell - Prefer C:\Program Files\WindowsApps instead of LocalAppData for pwsh detection #3223

Copilot generated summary

Provide a Copilot generated summary of the changes in this pull request.

Copilot summary

This pull request improves the handling of PowerShell executable detection, specifically addressing issues with Microsoft Store installations, and removes unused dialog-related dependencies from the PowerShellHostViewModel and its associated view. The main focus is to ensure the application can correctly resolve the real path of PowerShell when installed via the Microsoft Store, and to simplify the codebase by removing unnecessary dialog coordinator logic.

PowerShell executable detection improvements:

• Added a workaround in PowerShellHostViewModel to detect when pwsh.exe is a proxy stub from a Microsoft Store installation (located in AppData\Local\Microsoft\WindowsApps) and resolve its actual installation path using a new helper method FindRealPwshPath. This addresses issues where the application could not find the real PowerShell executable. [1] [2]
• If the real path cannot be resolved, the logic falls back to searching for Windows PowerShell.

Removal of dialog-related dependencies:

• Removed the IDialogCoordinator dependency from the PowerShellHostViewModel constructor and class, as it is no longer used. [1] [2]
• Cleaned up related XAML by removing the MahApps.Metro dialogs namespace and the DialogParticipation.Register binding from PowerShellHostView.xaml. [1] [2]
• Removed the MahApps.Metro.Controls.Dialogs using directive and updated the instantiation of PowerShellHostViewModel to use the parameterless constructor in PowerShellHostView.xaml.cs. [1] [2]

To-Do

• Update changelog to reflect this changes

Contributing

By submitting this pull request, I confirm the following:

• I have read and understood the contributing guidelines and the code of conduct.
• I have have added my name, username or email to the contributors list or don't want to.
• The code or resource is compatible with the GNU General Public License v3.0.

[Copilot]

Pull request overview

This pull request addresses an issue where PowerShell themes and settings were not being applied correctly when PowerShell was installed via the Windows Store. The root cause is that the Windows Store installation creates a proxy stub in %LocalAppData%\Microsoft\WindowsApps\pwsh.exe that redirects to the actual executable in C:\Program Files\WindowsApps\, but theme settings are applied to the real binary's registry path.

Key changes:

• Added logic to detect Windows Store installations of PowerShell and resolve the actual executable path
• Implemented FindRealPwshPath method that executes PowerShell to query its real location
• Removed unused IDialogCoordinator dependency from PowerShellHostViewModel and related UI bindings

Reviewed changes

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

File	Description
Source/NETworkManager/ViewModels/PowerShellHostViewModel.cs	Added Windows Store detection logic and FindRealPwshPath helper method; removed IDialogCoordinator dependency from constructor
Source/NETworkManager/Views/PowerShellHostView.xaml.cs	Removed MahApps.Metro.Controls.Dialogs import and updated to use parameterless constructor
Source/NETworkManager/Views/PowerShellHostView.xaml	Removed dialog-related namespace and DialogParticipation.Register binding
Website/docs/changelog/next-release.md	Updated changelog to document the Windows Store path resolution fix and reorganized bug fix entries

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The string replacement logic appears redundant and potentially incorrect. Lines 632-637 perform multiple overlapping replacements for removing newlines and carriage returns:

• Replace(@"\\", @"\") - converts double backslashes to single (this may corrupt valid Windows paths)
• Replace(@"\r", string.Empty) and Replace(@"\n", string.Empty) - these won't match literal \r or \n in the string
• Multiple redundant Replace("\r\n", ...), Replace("\n", ...), Replace("\r", ...) calls

Simplify to:

output = output.Trim();

The output from (Get-Command pwsh).Source should already be a clean path without embedded newlines. The .Trim() call will handle any leading/trailing whitespace.

Suggested change

	output = output.Replace(@"\\", @"\")
	.Replace(@"\r", string.Empty)
	.Replace(@"\n", string.Empty)
	.Replace("\r\n", string.Empty)
	.Replace("\n", string.Empty)
	.Replace("\r", string.Empty)
	.Trim();
	output = output.Trim();

[Copilot]

The catch block silently swallows all exceptions without logging. This makes debugging difficult if FindRealPwshPath fails. Consider logging the exception:

catch (Exception ex)
{
    Log.Error($"Failed to resolve real pwsh path: {ex.Message}");
    return null;
}

[Copilot]

Potential resource leak: The Process object is disposed via using, but if process.WaitForExit() hangs or takes too long, this could block indefinitely. Consider adding a timeout:

if (!process.WaitForExit(5000)) // 5 second timeout
{
    process.Kill();
    return null;
}

Suggested change

	process.WaitForExit();
	// Wait for up to 5 seconds for the process to exit
	if (!process.WaitForExit(5000))
	{
	process.Kill();
	return null;
	}

[Copilot]

Logic issue: If FindRealPwshPath returns null (line 575), the code keeps the WindowsApps stub path in applicationFilePath and proceeds to set it in the settings (line 585). This means the app will still try to use the stub path instead of falling back to Windows PowerShell.

Consider updating the logic:

if (realPwshPath != null)
    applicationFilePath = realPwshPath;
else
{
    Log.Warn("Failed to resolve real pwsh path. Falling back to Windows PowerShell.");
    applicationFilePath = ApplicationHelper.Find(PowerShell.WindowsPowerShellFileName);
}

Suggested change

	if (realPwshPath != null)
	applicationFilePath = realPwshPath;
	if (realPwshPath != null)
	{
	applicationFilePath = realPwshPath;
	}
	else
	{
	Log.Warn("Failed to resolve real pwsh path. Falling back to Windows PowerShell.");
	applicationFilePath = ApplicationHelper.Find(PowerShell.WindowsPowerShellFileName);
	}

[Copilot]

The path check using .EndsWith() is case-sensitive and may not handle all variations of the path (e.g., lowercase drive letters, forward slashes). Consider using a more robust check:

if (!string.IsNullOrEmpty(applicationFilePath) && 
    applicationFilePath.Contains("\\AppData\\Local\\Microsoft\\WindowsApps\\", StringComparison.OrdinalIgnoreCase) &&
    applicationFilePath.EndsWith("pwsh.exe", StringComparison.OrdinalIgnoreCase))

This handles case variations and ensures we're matching the correct directory structure.

Suggested change

	if (applicationFilePath.EndsWith("AppData\\Local\\Microsoft\\WindowsApps\\pwsh.exe"))
	var normalizedPath = applicationFilePath?.Replace('/', '\\');
	if (!string.IsNullOrEmpty(normalizedPath) &&
	normalizedPath.EndsWith("AppData\\Local\\Microsoft\\WindowsApps\\pwsh.exe", StringComparison.OrdinalIgnoreCase))

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The string replacement logic is redundant and inefficient. Lines 643-647 replace \r, \n, and \r\n multiple times in different forms. Line 645 replaces \r\n which makes lines 646-647 redundant since they would have no effect. Consider simplifying this to a single set of replacements: first \r\n, then \r, then \n.

Suggested change

	.Replace(@"\r", string.Empty)
	.Replace(@"\n", string.Empty)
	.Replace("\r\n", string.Empty)
	.Replace("\n", string.Empty)
	.Replace("\r", string.Empty);
	.Replace("\r\n", string.Empty)
	.Replace("\r", string.Empty)
	.Replace("\n", string.Empty);

[Copilot]

[nitpick] Missing space after if keyword. Should be if (!process.WaitForExit(10000)) for consistency with C# formatting conventions.

Suggested change

	if(!process.WaitForExit(10000))
	if (!process.WaitForExit(10000))

[Copilot]

There's a potential resource leak here. If Process.Start(psi) returns null (which can happen if the process fails to start), the null reference will cause an exception before reaching the using statement's disposal. Additionally, if WaitForExit returns false and the process is killed, StandardError is never read, which could lead to deadlock if the error stream buffer fills up. Consider checking for null after Process.Start() and reading StandardError before or alongside StandardOutput.

Suggested change

	using Process process = Process.Start(psi);
	string output = process.StandardOutput.ReadToEnd();
	if(!process.WaitForExit(10000))
	{
	process.Kill();
	Log.Warn("Timeout while trying to resolve real pwsh path.");
	return null;
	}
	if (string.IsNullOrEmpty(output))
	return null;
	output = output.Replace(@"\\", @"\")
	.Replace(@"\r", string.Empty)
	.Replace(@"\n", string.Empty)
	.Replace("\r\n", string.Empty)
	.Replace("\n", string.Empty)
	.Replace("\r", string.Empty);
	return output.Trim();
	Process process = Process.Start(psi);
	if (process == null)
	{
	Log.Error("Failed to start pwsh process to resolve real pwsh path.");
	return null;
	}
	using (process)
	{
	// Read both output and error streams asynchronously to avoid deadlocks
	Task<string> outputTask = process.StandardOutput.ReadToEndAsync();
	Task<string> errorTask = process.StandardError.ReadToEndAsync();
	bool exited = process.WaitForExit(10000);
	if (!exited)
	{
	process.Kill();
	Log.Warn("Timeout while trying to resolve real pwsh path.");
	}
	// Ensure both output and error are read
	Task.WaitAll(outputTask, errorTask);
	string output = outputTask.Result;
	string error = errorTask.Result;
	if (!string.IsNullOrEmpty(error))
	{
	Log.Warn($"Error while resolving real pwsh path: {error}");
	}
	if (string.IsNullOrEmpty(output))
	return null;
	output = output.Replace(@"\\", @"\")
	.Replace(@"\r", string.Empty)
	.Replace(@"\n", string.Empty)
	.Replace("\r\n", string.Empty)
	.Replace("\n", string.Empty)
	.Replace("\r", string.Empty);
	return output.Trim();
	}

[Copilot]

Reading StandardOutput before calling WaitForExit() can cause a deadlock if the output buffer fills up. The process will block waiting for the buffer to be read, while this code blocks waiting for the process to exit. Use WaitForExit() first, then read the output, or read the output asynchronously.