Refactor the module path construction code to make it more robust and easier to maintain by daxian-dbw · Pull Request #26565 · PowerShell/PowerShell

daxian-dbw · 2025-12-03T00:27:49Z

PR Summary

There are 2 issues surfaced from #26536 that we need to address:

The error reporting in the method ReportExceptionFallback(Exception e, string header) does not write out stack trace of the exception, which makes it hard to diagnose failures like ShellCannotBeStarted.
The UpdatePath method used in module path construction doesn't handle the case where personalModulePathToUse may contain multiple paths separated by the PathSperator (user can define PSModulePath in powershell.config.json with such a value).

The fixes are:

Write out stack trace of the exception in ReportExceptionFallback(Exception e, string header), at the end.
Refactored the module path construction code to make it more robust and easier to maintain
- Removed the old existing UpdatePath method.
- Removed the old PathContainsSubstring method. Change to a difference approach that is much more efficient.
- Refactored the AddPath method and rename it to be UpdatePath
- Updated SetModulePath() to not blindly add a path separator when the environment variable PSModulePath is initially not set in all scopes.
- Call SetModulePath() in the static constructor of ModuleIntrinsics instead of the instance constructor. So, we only need to construct the module path once per process (instead of once per a new Runspace).

With the changes, PowerShell won't crash even if the personal module path is like C:\Users\marcv\OneDrive - company & name namepart2\Dokumente\PowerShell\Modules as reported in the issue.

PR Checklist

… easier to maintain

Copilot

Pull request overview

This PR refactors the module path construction code in PowerShell to fix issue #26536, which involves two main problems: insufficient error diagnostics in exception fallback handling and incorrect handling of multi-path PSModulePath values from configuration files. The refactoring moves module path initialization to a static constructor, consolidates path manipulation methods, and improves error reporting by including stack traces.

Key Changes

Moved module path initialization from instance constructor to static constructor for earlier, one-time initialization
Refactored UpdatePath method (previously split between AddToPath and a wrapper UpdatePath) to handle multiple semicolon/colon-separated paths in a single call
Enhanced ReportExceptionFallback to output complete stack traces including inner exceptions
Added test coverage for PSModulePath configuration with multiple paths

Reviewed changes

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

File	Description
test/powershell/Host/ConsoleHost.Tests.ps1	Added test for PSModulePath with multiple paths from config file; contains platform-specific path separator issues
src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs	Refactored module path construction: moved initialization to static constructor, consolidated UpdatePath logic, improved PathContainsSubstring with better return semantics
src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs	Enhanced exception reporting to include stack traces for debugging ShellCannotBeStarted errors; contains potential null reference issues

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

Copilot

Pull request overview

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

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

iSazonov

On top of ModuleIntrinsics.cs I see we define Tracer. It would be nice to add to the trace source and result path strings for more easy diagnostics.

iSazonov · 2025-12-11T05:11:53Z

+            // Local function to normalize a path by removing trailing directory separators.
+            static string NormalizePath(string path)
+            {
+                string normalizedPath = path.TrimEnd(Path.DirectorySeparatorChar);


Why not use NormalizePath method from FileSystemProvider? Then we will follow the basic PowerShell behavior regarding directory separators.
Otherwise, we could use Path.TrimEndingDirectorySeparator()

Path.TrimEndingDirectorySeparator doesn't remove duplicate trailing directory separators. It only removes the ending one. But I think it should be practically fine, and also, I favor over simplicity. We can get back to it if it turns out to be an issue.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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

iSazonov · 2025-12-12T04:11:07Z

Ready to merge.

daxian-dbw · 2025-12-12T17:43:23Z

@iSazonov Thanks for the review!

… easier to maintain (PowerShell#26565)

Print the stack trace of the ShellCannotBeStarted error

114803b

daxian-dbw mentioned this pull request Dec 3, 2025

Error when starting powershell 7.4, 7.5 or 7.6; "Index was out of range. Must be non-negative and less than or equal to the size of the collection. (Parameter “startIndex”)" #26536

Closed

5 tasks

daxian-dbw added 2 commits December 3, 2025 15:57

Add more debug messages

ad1aa09

Guard the debug message with env var

64acf5c

microsoft-github-policy-service Bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 7, 2025

Refactor the module path construction code to make it more robust and…

866ee21

… easier to maintain

daxian-dbw changed the title ~~[debug] Print the stack trace of the ShellCannotBeStarted error~~ Refactor the module path construction code to make it more robust and easier to maintain Dec 9, 2025

daxian-dbw added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Dec 9, 2025

microsoft-github-policy-service Bot removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Dec 9, 2025

daxian-dbw marked this pull request as ready for review December 10, 2025 00:07

daxian-dbw requested review from SeeminglyScience, adityapatwardhan and Copilot December 10, 2025 00:07

Copilot started reviewing on behalf of daxian-dbw December 10, 2025 00:07 View session

Copilot AI reviewed Dec 10, 2025

View reviewed changes

Address comments from Copilot

96e7a32

iSazonov reviewed Dec 10, 2025

View reviewed changes

Address comments and check if a path already exists more efficiently

45c1d4c

daxian-dbw requested a review from Copilot December 10, 2025 19:48

Copilot started reviewing on behalf of daxian-dbw December 10, 2025 19:49 View session

Copilot AI reviewed Dec 10, 2025

View reviewed changes

Update the XML doc for a parameter based on Copilot's feedback

fd2ce82

iSazonov reviewed Dec 11, 2025

View reviewed changes

Address Ilya's comments

5610fc3

iSazonov requested a review from Copilot December 11, 2025 19:18

Copilot started reviewing on behalf of iSazonov December 11, 2025 19:19 View session

Copilot AI reviewed Dec 11, 2025

View reviewed changes

iSazonov reviewed Dec 11, 2025

View reviewed changes

Comment thread test/powershell/Host/ConsoleHost.Tests.ps1 Outdated

Address a comment

2af8081

iSazonov approved these changes Dec 12, 2025

View reviewed changes

daxian-dbw merged commit 592668b into PowerShell:master Dec 12, 2025
36 checks passed

daxian-dbw deleted the debug branch December 12, 2025 17:43

kilasuit pushed a commit to kilasuit/PowerShell that referenced this pull request Jan 2, 2026

Refactor the module path construction code to make it more robust and…

54483ac

… easier to maintain (PowerShell#26565)

Conversation

daxian-dbw commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iSazonov Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

iSazonov commented Dec 12, 2025

Uh oh!

Uh oh!

daxian-dbw commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

daxian-dbw commented Dec 3, 2025 •

edited

Loading

daxian-dbw Dec 11, 2025 •

edited

Loading