Skip to content

Fix splitting pane fails when CWD is a PSDrive (#20373)#20379

Open
tarunbtw wants to merge 1 commit into
microsoft:mainfrom
tarunbtw:fix/20373-psdrive-split
Open

Fix splitting pane fails when CWD is a PSDrive (#20373)#20379
tarunbtw wants to merge 1 commit into
microsoft:mainfrom
tarunbtw:fix/20373-psdrive-split

Conversation

@tarunbtw

Copy link
Copy Markdown

Summary of the Pull Request

Fixes pane splitting failing when the active shell's working directory is a PowerShell PSDrive path instead of a real filesystem path.

References and Relevant Issues

Fixes #20373

Detailed Description of the Pull Request / Additional comments

When splitting a pane, TerminalPage reads the current pane's working directory from control.WorkingDirectory() and uses it as the new pane's starting directory if Utils::IsValidDirectory() returns true.

PowerShell reports its CWD through shell integration using $PWD.Path. For a PSDrive (created with New-PSDrive -Name src -Root C:\src), this returns the provider path src:\ instead of the real filesystem path. IsValidDirectory() only ran GetFileAttributesExW on the string without checking that it is a well formed Win32 path, so src:\ passed validation and was handed to ConptyConnection, then to CreateProcess's lpCurrentDirectory, where it resolved relative to the host process's own CWD instead of failing cleanly. That produced the launch error in the original report.

This PR adds a check in IsValidDirectory that requires the path to be a single drive letter path (C:...) or a UNC path (\server\share...) before checking the filesystem. Provider paths like src:\ now fail validation immediately, so the split falls back to the profile's default starting directory instead of launching with a broken one.

PR Checklist

@tarunbtw

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@DHowett DHowett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, so, PowerShell doesn't come with shell integration.

Wouldn't it be easier for the user who filed the bug to fix their profile to emit $PWD.ProviderPath instead of $PWD?

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 30, 2026
@tarunbtw

Copy link
Copy Markdown
Author

That's a fair point, and yeah, the doc snippet at learn.microsoft.com/en-us/windows/terminal/tutorials/shell-integration uses $loc.ToString() (provider path) instead of $loc.ProviderPath, which is the direct cause of this specific repro. I'll look into filing/fixing that separately.

That said, I'd like to keep this validation in IsValidDirectory regardless. Terminal currently trusts any string a shell integration script reports as CWD and passes it straight to CreateProcess's lpCurrentDirectory. That's not specific to the official PowerShell snippet, OSC 9;9 can come from oh-my-posh themes or any custom prompt, and any of them could emit a non-Win32 path the same way. Right now that fails ungracefully (the garbled WindowsApps path in the original report); with this change it falls back to the profile's default starting directory instead.

Happy to narrow the PR if you'd rather this be handled purely as a docs fix, just let me know.

@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 30, 2026
@DHowett

DHowett commented Jun 30, 2026

Copy link
Copy Markdown
Member

(We'll need to comport it with #20094 as well)

@tarunbtw

tarunbtw commented Jun 30, 2026

Copy link
Copy Markdown
Author

Yeah, looked closer and you're right, #20094 drops the IsValidDirectory check at both call sites in TerminalPage.cpp and just checks !empty() instead, so hardening IsValidDirectory alone won't help once that lands, those sites won't even call it anymore.

The only place both old and new code actually share is the OSC 9;9 parsing in adaptDispatch.cpp. Right now til::is_legal_path() just filters illegal characters, it doesn't care about format, so src:\ goes straight through into _workingDirectory.

Can't just slap a strict Win32-path check there either, ~ needs to keep working since MangleStartingDirectoryForWSL relies on it as a placeholder. Would need something narrower, reject word:\ where word isn't a single drive letter, leave UNC paths and ~ alone.

Want me to take a pass at that, or hold off until #20094 settles so I'm not stepping on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs-Attention The core contributors need to come back around and look at this ASAP.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spliiting terminal from with PSDrive fails

2 participants