Add -NoProfile to launched pwsh consoles

[sharpninja]

Summary

• add -NoProfile to every pwsh launch path used by start_console
• route Windows, macOS, Linux terminal, and Linux headless launch arguments through shared helpers
• add unit coverage for the generated launch commands/arguments

Validation

• dotnet test Tests/PowerShell.MCP.Tests.csproj --configuration Release --no-restore
• dotnet build PowerShell.MCP.sln --configuration Release --no-restore

Summary by CodeRabbit

• Refactor

  • Consolidated PowerShell launch mechanisms across Windows, macOS, and Linux platforms with shared command-line argument builders for improved consistency and maintainability.

• Tests

  • Added test cases validating consistent PowerShell launch behavior across Windows, macOS, Linux, and headless execution modes.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR consolidates PowerShell launcher command-line construction across platform-specific implementations. A shared NoProfileArgument constant and builder methods for Windows, macOS, Linux, and headless launchers are introduced, and all launcher code paths are updated to call these builders instead of maintaining inline literal strings. Tests validate consistent -NoProfile handling across all invocation routes.

Changes

Shared Launcher Command-Line Builders

Layer / File(s)	Summary
Shared launcher helper builders and constants PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs	New PwshLauncherShared constant NoProfileArgument (-NoProfile) and builder methods BuildWindowsCommandLine, BuildMacOSDoScriptCommand, BuildLinuxPwshCommand, and BuildHeadlessPwshArguments are introduced to centralize command-line construction logic.
Platform-specific launcher refactoring PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs	Windows, macOS, Linux, and headless launcher code paths are refactored to call shared builder methods instead of constructing command-line strings inline, replacing hardcoded pwsh.exe, pwsh -NoExit -File, exec pwsh -NoExit -EncodedCommand, and ArgumentList construction.
Shared builder validation tests Tests/Unit/Proxy/PwshLauncherSharedTests.cs	New unit tests assert that all shared builders consistently include -NoProfile with correct argument ordering (-NoProfile -NoExit -Command for Windows/headless, -NoProfile -NoExit -File for macOS, exec pwsh -NoProfile -NoExit -EncodedCommand for Linux).

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 Platform-specific launchers now share common helpers,

No more -NoProfile copy-paste blunders!

From Windows to Linux, one builder rules them all,

Testing ensures consistency through each pwsh call. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly summarizes the main change: adding the -NoProfile flag to pwsh console launches across all platforms.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs (1)

316-325: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Windows launcher missing critical agentId escaping and startLocation handling.

The Windows launcher builds its command inline (lines 316-324) instead of using BuildInitCommand, causing several inconsistencies with other platforms:

1. Critical: Line 319 and 323 construct $global:PowerShellMCPAgentId = '{agentId}' without escaping single quotes. If agentId contains a single quote (e.g., "test'id"), the resulting PowerShell will be malformed: $global:PowerShellMCPAgentId = 'test'id', causing a syntax error or potential command injection.

2. Major: The startLocation parameter is ignored in the command construction (only used as lpCurrentDirectory in CreateProcessW on line 335), while BuildInitCommand includes Set-Location -LiteralPath with proper escaping (lines 205-207 in BuildInitCommand). This creates inconsistent behavior across platforms.

3. Major: Missing ModuleCaseFix (line 210 in BuildInitCommand) that handles case-sensitive file systems.

🔧 Proposed fix to use BuildInitCommand for consistency

         var proxyPid = Process.GetCurrentProcess().Id;
-        string command;
-        if (!string.IsNullOrEmpty(startupCommands))
-        {
-            command = $"{PwshLauncherShared.EncodingPrelude}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; Import-Module PowerShell.MCP -Force; Import-Module PSReadLine; {startupCommands}";
-        }
-        else
-        {
-            command = $"{PwshLauncherShared.EncodingPrelude}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; Import-Module PowerShell.MCP -Force; Import-Module PSReadLine";
-        }
+        var initCommand = PwshLauncherShared.BuildInitCommand(proxyPid, agentId, startupCommands, startLocation);
+        // Windows imports PSReadLine explicitly (other platforms remove it to avoid conflicts)
+        var command = $"{initCommand}; Import-Module PSReadLine";
         string commandLine = PwshLauncherShared.BuildWindowsCommandLine(command);

Note: This changes Windows to remove PSReadLine then re-import it. If that's undesirable, add a parameter to BuildInitCommand to control PSReadLine handling, or add the PSReadLine import in BuildInitCommand conditionally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs` around lines 316 -
325, Replace the inline Windows command construction in PowerShellProcessManager
(the block that sets `command` and calls
`PwshLauncherShared.BuildWindowsCommandLine`) with a call to the existing
BuildInitCommand so agentId is properly escaped and startLocation and
ModuleCaseFix are applied; specifically, call BuildInitCommand with the same
inputs used elsewhere (agentId, proxyPid, startupCommands, startLocation) to
produce the init command (it already applies literal-path Set-Location, escapes
quotes, and handles ModuleCaseFix), then pass the resulting string through
PwshLauncherShared.EncodingPrelude/BuildWindowsCommandLine as before and remove
the duplicated manual `Import-Module`/PSReadLine logic (or if PSReadLine must
differ on Windows, add a parameter to BuildInitCommand to control PSReadLine
behavior and use that).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs`:
- Around line 316-325: Replace the inline Windows command construction in
PowerShellProcessManager (the block that sets `command` and calls
`PwshLauncherShared.BuildWindowsCommandLine`) with a call to the existing
BuildInitCommand so agentId is properly escaped and startLocation and
ModuleCaseFix are applied; specifically, call BuildInitCommand with the same
inputs used elsewhere (agentId, proxyPid, startupCommands, startLocation) to
produce the init command (it already applies literal-path Set-Location, escapes
quotes, and handles ModuleCaseFix), then pass the resulting string through
PwshLauncherShared.EncodingPrelude/BuildWindowsCommandLine as before and remove
the duplicated manual `Import-Module`/PSReadLine logic (or if PSReadLine must
differ on Windows, add a parameter to BuildInitCommand to control PSReadLine
behavior and use that).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0ddf34e6-b286-4041-ba81-9fdeb49a1595

📥 Commits

Reviewing files that changed from the base of the PR and between ddb8342 and c2f87be.

📒 Files selected for processing (2)

• PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs
• Tests/Unit/Proxy/PwshLauncherSharedTests.cs

[yotsuda]

Thanks for this, @sharpninja — clean cross-platform work and I appreciate the test coverage on the launch argument builders.

I want to be transparent about how I'm thinking about it, because the change bundles a refactor with a behavior change and I land differently on the two halves.

On -NoProfile — partially yes, partially no.

The launched consoles in this project aren't only automation targets. On Windows, macOS, and Linux, start_console opens a real, human-facing interactive shell that the user actually looks at and types into — that's a deliberate product choice, not an implementation detail. Suppressing the user's $PROFILE there silently strips their prompt, aliases, PSReadLine predictors/keybindings, and theme. For those three interactive paths I'd rather keep loading the user profile.

The headless path (LaunchPwshDirectly) is different — that one is genuinely automation/CI, where -NoProfile is an unambiguous win: faster startup, deterministic init, no banner noise, no risk of a profile interfering with the encoding/PSReadLine setup. I'd happily take -NoProfile scoped to LaunchPwshDirectly only.

So my ask: could you narrow this PR to apply -NoProfile to the headless launcher path, and leave the Windows/macOS/Linux interactive launchers loading the profile as before? If you (or others) have a concrete case where a user profile breaks the interactive console init, I'm open to revisiting — ideally behind an explicit opt-out env toggle rather than a blanket default — but I'd want that driven by a real repro.

On the refactor itself.

Each of the new builder methods has a single call site, so this is more indirection than de-duplication — there wasn't actually repeated launch-string code to consolidate (the four platforms each have a distinct command shape). I'm not opposed to a small shared helper, but I'd keep it minimal; happy to just take the headless -NoProfile change with little or no extraction if that's simpler.

Thanks again for the contribution — happy to merge once the -NoProfile scope is narrowed.

[sharpninja]

How about making it optional? Driven by env variable? Cache the environment var on first use to static to avoid repeated registry calls. If we make it optional, then the new method becomes necessary.

[yotsuda]

@sharpninja what's the actual motivation for -NoProfile? The PR doesn't say what problem it solves. Did a profile actually break something?

I can't change the default to -NoProfile on the interactive paths — those are real shells, and it'd silently strip the user's prompt, aliases, PSReadLine, and theme. Headless might be fine, but I'd want to hear the reasoning first either way.