Skip to content

Commit f6f95b1

Browse files
yotsudaclaude
andcommitted
refactor(proxy): single BuildInitCommand for every non-Windows launcher
The PowerShell init script that bootstraps PowerShell.MCP into a freshly launched pwsh used to live in three near-identical inline copies. macOS extracted it to PwshLauncherMacOS.BuildInitCommand back when issue #45 was fixed (see April 2026 work) but the Linux terminal path and the Linux headless path each kept their own inline construction, drifted in two ways: - agentId went into the script unescaped — `IsValidAgentId` keeps the current ID format quote-free in practice, but the safety net the macOS path has (escapedAgentId) was missing on Linux, so a future ID format change would break only the Linux launches. - The headless path skipped Set-Location and Remove-Module PSReadLine entirely. Set-Location was substituted with -WorkingDirectory, but the PSReadLine cleanup was just absent. Move BuildInitCommand from PwshLauncherMacOS into PwshLauncherShared so all three call sites (macOS tempFile, Linux Base64, Linux headless ArgumentList) build the script body the same way. Each platform keeps its own delivery mechanism for documented reasons: Windows pwsh.exe -Command "<inline>" (Win32, no shell layer) macOS pwsh -File '/tmp/.../init.ps1' (AppleScript do script echoes the command line — Base64 would dump 1.2KB of noise into Terminal.app) Linux pwsh -EncodedCommand <base64> (setsid <terminal> -- <shell> -l -c '<pwsh>' multi-shell quoting through `sh -c` is the original reason for the encoding hop) Linux pwsh -Command <init> (headless / ArgumentList path, no shell layer at all) Renamed the existing PwshLauncherMacOSTests file to PwshLauncherSharedTests since the helper is no longer macOS-specific. The 4 quoting / case-fix regression tests now cover every platform that calls BuildInitCommand, and a single change to the script body will cause exactly one test file to fail loudly instead of silently diverging across launchers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bc53485 commit f6f95b1

3 files changed

Lines changed: 94 additions & 101 deletions

File tree

PowerShell.MCP.Proxy/Services/PowerShellProcessManager.cs

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,24 @@ internal static class PwshLauncherShared
174174
// may create the module directory as 'powershell.mcp'. Rename it so Import-Module can
175175
// locate the PascalCase name. No-op on case-insensitive file systems.
176176
internal const string ModuleCaseFix = "foreach ($p in ($env:PSModulePath -split [IO.Path]::PathSeparator)) { if ([string]::IsNullOrWhiteSpace($p)) { continue }; $lc = Join-Path $p 'powershell.mcp'; $uc = Join-Path $p 'PowerShell.MCP'; if ((Test-Path $lc) -and -not (Test-Path $uc)) { Rename-Item $lc $uc; break } }; ";
177+
178+
// Builds the PowerShell initialization command shared by every non-Windows launcher.
179+
// The actual delivery to pwsh differs per platform — macOS writes this to /tmp and
180+
// launches with `-File`, Linux Base64-encodes it for `-EncodedCommand` because of
181+
// shell quoting through `sh -c` — but the script body is identical. Single quotes
182+
// inside agentId / startLocation are escaped per PowerShell's '' convention so a
183+
// future ID format change can't break the launch line.
184+
// Kept internal so unit tests can lock in shell-safety without spawning a process.
185+
internal static string BuildInitCommand(int proxyPid, string agentId, string? startupCommands, string? startLocation)
186+
{
187+
var setLocation = string.IsNullOrEmpty(startLocation)
188+
? "Set-Location ~; "
189+
: $"Set-Location -LiteralPath '{startLocation.Replace("'", "''")}'; ";
190+
191+
var escapedAgentId = agentId.Replace("'", "''");
192+
var core = $"{setLocation}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{escapedAgentId}'; {ModuleCaseFix}Import-Module PowerShell.MCP -Force; Remove-Module PSReadLine -ErrorAction SilentlyContinue";
193+
return string.IsNullOrEmpty(startupCommands) ? core : $"{core}; {startupCommands}";
194+
}
177195
}
178196

179197
/// <summary>
@@ -326,7 +344,7 @@ public static void LaunchPwsh(string agentId, string? startupCommands = null, st
326344
};
327345

328346
var proxyPid = Process.GetCurrentProcess().Id;
329-
var initCommand = BuildInitCommand(proxyPid, agentId, startupCommands, startLocation);
347+
var initCommand = PwshLauncherShared.BuildInitCommand(proxyPid, agentId, startupCommands, startLocation);
330348

331349
// Write the init to a temp .ps1 and launch with `-File`. AppleScript's `do script`
332350
// types its argument into the Terminal window verbatim — if we passed the PS source
@@ -351,18 +369,6 @@ public static void LaunchPwsh(string agentId, string? startupCommands = null, st
351369
}
352370
}
353371

354-
// Builds the PowerShell initialization script written to a temp .ps1 and loaded via `pwsh -File`.
355-
// Kept internal so unit tests can lock in shell-safety (no unescaped quotes) without spawning a process.
356-
internal static string BuildInitCommand(int proxyPid, string agentId, string? startupCommands, string? startLocation)
357-
{
358-
var setLocation = string.IsNullOrEmpty(startLocation)
359-
? "Set-Location ~; "
360-
: $"Set-Location -LiteralPath '{startLocation.Replace("'", "''")}'; ";
361-
362-
var escapedAgentId = agentId.Replace("'", "''");
363-
var core = $"{setLocation}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{escapedAgentId}'; {PwshLauncherShared.ModuleCaseFix}Import-Module PowerShell.MCP -Force; Remove-Module PSReadLine -ErrorAction SilentlyContinue";
364-
return string.IsNullOrEmpty(startupCommands) ? core : $"{core}; {startupCommands}";
365-
}
366372
}
367373

368374
/// <summary>
@@ -437,27 +443,12 @@ private static bool TryLaunchTerminal(string terminal, string agentId, string? s
437443
CreateNoWindow = true
438444
};
439445

440-
// Build initialization command and encode to Base64 to avoid shell quoting issues
441-
// Set global variables with proxy PID and agent ID before importing module
446+
// Build the PowerShell init script (shared with macOS) and encode to Base64.
447+
// Linux delivers it via -EncodedCommand because the launch path goes through
448+
// `setsid <terminal> ... <shell> -l -c '<pwshCommand>'` and quoting through
449+
// multiple shell layers is fragile; -EncodedCommand sidesteps that entirely.
442450
var proxyPid = Process.GetCurrentProcess().Id;
443-
var caseFix = PwshLauncherShared.ModuleCaseFix;
444-
445-
// Set working directory via Set-Location inside the command to avoid shell quoting issues
446-
var setLocation = string.IsNullOrEmpty(startLocation)
447-
? "Set-Location ~; "
448-
: $"Set-Location -LiteralPath '{startLocation.Replace("'", "''")}'; ";
449-
450-
string initCommand;
451-
if (!string.IsNullOrEmpty(startupCommands))
452-
{
453-
initCommand = $"{setLocation}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; {caseFix}Import-Module PowerShell.MCP -Force; Remove-Module PSReadLine -ErrorAction SilentlyContinue; {startupCommands}";
454-
}
455-
else
456-
{
457-
initCommand = $"{setLocation}$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; {caseFix}Import-Module PowerShell.MCP -Force; Remove-Module PSReadLine -ErrorAction SilentlyContinue";
458-
}
459-
460-
// Encode command to Base64 (UTF-16LE) to bypass shell quoting/expansion issues
451+
var initCommand = PwshLauncherShared.BuildInitCommand(proxyPid, agentId, startupCommands, startLocation);
461452
var encodedCommand = Convert.ToBase64String(System.Text.Encoding.Unicode.GetBytes(initCommand));
462453

463454
// Command to launch pwsh with encoded initialization via login shell
@@ -537,15 +528,14 @@ private static bool TryLaunchTerminal(string terminal, string agentId, string? s
537528
/// </summary>
538529
private static void LaunchPwshDirectly(string agentId, string? startupCommands, string? startLocation)
539530
{
531+
// Reuse the shared init builder so a single source of truth (with proper
532+
// single-quote escaping) covers terminal / headless / macOS paths. ArgumentList
533+
// delivers -Command as a separate argv entry, so no Base64 encoding is needed
534+
// here even though the terminal-launched Linux path uses -EncodedCommand.
535+
// BuildInitCommand prepends Set-Location, so the headless cwd is established
536+
// by the script body itself; -WorkingDirectory is no longer needed.
540537
var proxyPid = Process.GetCurrentProcess().Id;
541-
var caseFix = PwshLauncherShared.ModuleCaseFix;
542-
var initCommand = string.IsNullOrEmpty(startupCommands)
543-
? $"$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; {caseFix}Import-Module PowerShell.MCP -Force"
544-
: $"$global:PowerShellMCPProxyPid = {proxyPid}; $global:PowerShellMCPAgentId = '{agentId}'; {caseFix}Import-Module PowerShell.MCP -Force; {startupCommands}";
545-
546-
var workingDir = string.IsNullOrEmpty(startLocation)
547-
? Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)
548-
: startLocation;
538+
var initCommand = PwshLauncherShared.BuildInitCommand(proxyPid, agentId, startupCommands, startLocation);
549539

550540
var psi = new ProcessStartInfo
551541
{
@@ -557,8 +547,6 @@ private static void LaunchPwshDirectly(string agentId, string? startupCommands,
557547
RedirectStandardError = true,
558548
};
559549
psi.ArgumentList.Add("-NoExit");
560-
psi.ArgumentList.Add("-WorkingDirectory");
561-
psi.ArgumentList.Add(workingDir);
562550
psi.ArgumentList.Add("-Command");
563551
psi.ArgumentList.Add(initCommand);
564552

Tests/Unit/Proxy/PwshLauncherMacOSTests.cs

Lines changed: 0 additions & 58 deletions
This file was deleted.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
using PowerShell.MCP.Proxy.Services;
2+
using Xunit;
3+
4+
namespace PowerShell.MCP.Tests.Unit.Proxy;
5+
6+
// PwshLauncherShared.BuildInitCommand is the single source of truth for the
7+
// PowerShell init script that all non-Windows launchers run when starting a
8+
// fresh pwsh session. The original regression (GitHub issue #45) was a broken
9+
// '' quoting on macOS that produced `$global:PowerShellMCPAgentId = ''default''`,
10+
// which zsh collapsed to a bareword `default`. After the helper was extracted
11+
// from PwshLauncherMacOS into PwshLauncherShared so PwshLauncherLinux can use
12+
// it too, these tests guard the PowerShell-level quote escaping for every
13+
// platform that delivers the script via pwsh (whether through `-File` /
14+
// `-EncodedCommand` / `-Command`).
15+
public class PwshLauncherSharedTests
16+
{
17+
private const string DefaultAgentId = "default";
18+
private const int DefaultPid = 12345;
19+
20+
[Fact]
21+
public void BuildInitCommand_DefaultAgent_ContainsNoDoubleSingleQuotes()
22+
{
23+
var cmd = PwshLauncherShared.BuildInitCommand(DefaultPid, DefaultAgentId, null, null);
24+
25+
Assert.DoesNotContain("''default''", cmd);
26+
Assert.Contains("$global:PowerShellMCPAgentId = 'default'", cmd);
27+
}
28+
29+
[Fact]
30+
public void BuildInitCommand_StartLocationWithSingleQuote_IsEscapedForPowerShell()
31+
{
32+
// PowerShell single-quoted string escape is '' (doubling). This is a
33+
// *PowerShell* concern — independent of how the script body reaches pwsh
34+
// (temp .ps1 on macOS, Base64 on Linux, ArgumentList on the headless path).
35+
var cmd = PwshLauncherShared.BuildInitCommand(DefaultPid, DefaultAgentId, null, "/Users/o'brien");
36+
37+
Assert.Contains("Set-Location -LiteralPath '/Users/o''brien';", cmd);
38+
}
39+
40+
[Fact]
41+
public void BuildInitCommand_AgentIdWithSingleQuote_IsEscapedForPowerShell()
42+
{
43+
var cmd = PwshLauncherShared.BuildInitCommand(DefaultPid, "te'st", null, null);
44+
45+
Assert.Contains("$global:PowerShellMCPAgentId = 'te''st'", cmd);
46+
}
47+
48+
[Fact]
49+
public void BuildInitCommand_IncludesModuleCaseFixBeforeImport()
50+
{
51+
// Case-sensitive file systems (Linux ext4/btrfs, case-sensitive APFS)
52+
// can surface a lowercase 'powershell.mcp' directory that
53+
// Install-PSResource creates. The case-fix script must run before
54+
// Import-Module so the PascalCase name resolves.
55+
var cmd = PwshLauncherShared.BuildInitCommand(DefaultPid, DefaultAgentId, null, null);
56+
57+
var caseFixIdx = cmd.IndexOf("Rename-Item $lc $uc", StringComparison.Ordinal);
58+
var importIdx = cmd.IndexOf("Import-Module PowerShell.MCP -Force", StringComparison.Ordinal);
59+
60+
Assert.True(caseFixIdx >= 0, "case-fix snippet must be present");
61+
Assert.True(caseFixIdx < importIdx, "case-fix must run before Import-Module");
62+
}
63+
}

0 commit comments

Comments
 (0)