Skip to content

fix: Setup no longer reports optional npm cleanup failures #1452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hatayama merged 6 commits into from
Jul 1, 2026
Merged

fix: Setup no longer reports optional npm cleanup failures #1452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7271c16
Silence legacy npm cleanup failures
hatayama Jul 1, 2026
9242799
Preserve Windows path shadow warnings
hatayama Jul 1, 2026
3c652a0
Remove README cleanup guidance changes
hatayama Jul 1, 2026
a4e82ea
Bump dispatcher release version
hatayama Jul 1, 2026
09e73c6
Normalize Windows PATH shadow probes
hatayama Jul 1, 2026
0b21cdf
Preserve Windows drive-root PATH probes
hatayama Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/dispatcher-contract.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"schemaVersion": 1,
"dispatcherVersion": "3.0.1-beta.8",
"dispatcherVersion": "3.0.1-beta.9",
"dispatcherContractVersion": 1
}
58 changes: 12 additions & 46 deletions cli/internal/install/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,21 +122,6 @@ function Test-LegacyNpmUloopPath {
function Write-LegacyNpmMultilineArgumentWarning {
Write-Host 'Legacy npm shims can alter multiline PowerShell arguments before the native CLI receives them.'
}
function Write-LegacyNpmManualRemoval {
param([string]$LegacyUloopPath, [string]$LegacyPrefix)
Write-Host 'Could not remove the legacy npm package automatically.'
Write-LegacyNpmMultilineArgumentWarning
if ($LegacyUloopPath) {
Write-Host "Legacy uloop command: $LegacyUloopPath"
}
if ($LegacyPrefix) {
Write-Host 'Run this manually if that command still shadows the native CLI:'
Write-Host (' npm uninstall -g --prefix "' + $LegacyPrefix + '" uloop-cli')
return
}
Write-Host 'Run this manually if the old npm command still shadows the native CLI:'
Write-Host ' npm uninstall -g uloop-cli'
}
function Remove-LegacyNpmArtifacts {
param([string]$LegacyUloopPath, [string]$LegacyPrefix)
if ($LegacyUloopPath -and (Test-Path $LegacyUloopPath -PathType Leaf)) {
Expand Down Expand Up @@ -193,7 +178,6 @@ function Invoke-LegacyNpmPackageRemoval {
$LegacyPrefix = Get-NpmPrefixFromUloopPath -CommandPath $LegacyUloopPath
}
if (-not $LegacyPrefix) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix
return $false
}
if ($NpmCommand) {
Expand All @@ -204,7 +188,6 @@ function Invoke-LegacyNpmPackageRemoval {
Remove-LegacyNpmArtifacts -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix
}
if (Test-LegacyNpmArtifactsExist -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix
return $false
}
Write-Host 'Removed legacy npm package: uloop-cli'
Expand Down Expand Up @@ -253,20 +236,10 @@ function Get-LegacyNpmUloopPathsFromPath {
}
function Invoke-AllLegacyNpmPackageRemoval {
param([string]$ExpectedUloopPath)
$RemovedAll = $true
foreach ($LegacyUloopPath in (Get-LegacyNpmUloopPathsFromPath)) {
if (-not (Invoke-LegacyNpmPackageRemoval -LegacyUloopPath $LegacyUloopPath -ExpectedUloopPath $ExpectedUloopPath)) {
$RemovedAll = $false
}
Invoke-LegacyNpmPackageRemoval -LegacyUloopPath $LegacyUloopPath -ExpectedUloopPath $ExpectedUloopPath | Out-Null
}
Invoke-DefaultLegacyNpmPackageRemoval
foreach ($LegacyUloopPath in (Get-LegacyNpmUloopPathsFromPath)) {
if (-not [string]::Equals($LegacyUloopPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix (Get-NpmPrefixFromUloopPath -CommandPath $LegacyUloopPath)
$RemovedAll = $false
}
}
return $RemovedAll
}
function Set-UserPathWithInstallDirectoryFirst {
param([string]$Directory)
Expand Down Expand Up @@ -296,48 +269,41 @@ function Get-FirstUloopCommandFromPath {
return $null
}
foreach ($PathEntry in ($PathValue -split ';')) {
if (-not $PathEntry) {
$NormalizedPathEntry = & $NormalizePath $PathEntry
if (-not $NormalizedPathEntry) {
continue
}
if ($NormalizedPathEntry -match '^[A-Za-z]:$') {
$NormalizedPathEntry = $NormalizedPathEntry + '\'
}
foreach ($ShimName in @('uloop.exe', 'uloop.cmd', 'uloop.ps1', 'uloop')) {
$CandidatePath = Join-Path $PathEntry $ShimName
$CandidatePath = Join-Path $NormalizedPathEntry $ShimName
if (Test-Path $CandidatePath -PathType Leaf) {
return $CandidatePath
}
}
}
return $null
}
function Test-PersistedUloopCommandResolvesToExpectedPath {
param([string]$ExpectedUloopPath)
function Report-PathShadowing {
$MachinePath = [Environment]::GetEnvironmentVariable('Path', 'Machine')
$UserPath = [Environment]::GetEnvironmentVariable('Path', 'User')
$ResolvedPath = Get-FirstUloopCommandFromPath -PathValue ([string]::Join(';', @($MachinePath, $UserPath)))
if (-not $ResolvedPath) {
return $false
}
return [string]::Equals($ResolvedPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)
}
function Report-PathShadowing {
$ResolvedCommand = Get-Command uloop -ErrorAction SilentlyContinue | Select-Object -First 1
if (-not $ResolvedCommand) {
return
}
if ([string]::Equals($ResolvedCommand.Source, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
if ([string]::Equals($ResolvedPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
return
}
Write-Host "Installed uloop to $ExpectedUloopPath, but PATH resolves uloop to:"
Write-Host " $($ResolvedCommand.Source)"
if (Test-LegacyNpmUloopPath -CommandPath $ResolvedCommand.Source) {
Write-Host " $ResolvedPath"
if (Test-LegacyNpmUloopPath -CommandPath $ResolvedPath) {
Write-LegacyNpmMultilineArgumentWarning
}
Write-Host "Move $InstallDir earlier in PATH, or remove the legacy installation if it owns that command."
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Set-UserPathWithInstallDirectoryFirst -Directory $InstallDir
$LegacyCleanupSucceeded = Invoke-AllLegacyNpmPackageRemoval -ExpectedUloopPath $ExpectedUloopPath
if ((-not $LegacyCleanupSucceeded) -and (-not (Test-PersistedUloopCommandResolvesToExpectedPath -ExpectedUloopPath $ExpectedUloopPath))) {
throw 'Failed to remove the legacy npm uloop-cli package.'
}
Invoke-AllLegacyNpmPackageRemoval -ExpectedUloopPath $ExpectedUloopPath | Out-Null
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Report-PathShadowing
`,
powerShellSingleQuote(installDir),
Expand Down
38 changes: 26 additions & 12 deletions cli/internal/install/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,17 @@ func TestCommandForWindowsConfiguresUserPathAndLegacyCleanup(t *testing.T) {
"GetExtension($CommandPath), '.exe'",
"foreach ($ShimName in @('uloop', 'uloop.cmd', 'uloop.ps1'))",
"Invoke-AllLegacyNpmPackageRemoval -ExpectedUloopPath $ExpectedUloopPath",
"npm uninstall -g --prefix",
"npm uninstall -g uloop-cli",
"$NpmArgs = @('uninstall', '-g', '--prefix', $LegacyPrefix, 'uloop-cli')",
"$null = & $NpmCommand.Source @('uninstall', '-g', 'uloop-cli')",
"Report-PathShadowing",
"function Get-FirstUloopCommandFromPath",
"$NormalizedPathEntry = & $NormalizePath $PathEntry",
"if ($NormalizedPathEntry -match '^[A-Za-z]:$') {\n $NormalizedPathEntry = $NormalizedPathEntry + '\\'\n }",
"$CandidatePath = Join-Path $NormalizedPathEntry $ShimName",
"$MachinePath = [Environment]::GetEnvironmentVariable('Path', 'Machine')",
"$ResolvedPath = Get-FirstUloopCommandFromPath -PathValue ([string]::Join(';', @($MachinePath, $UserPath)))",
"function Write-LegacyNpmMultilineArgumentWarning",
"if (Test-LegacyNpmUloopPath -CommandPath $ResolvedCommand.Source) {\n Write-LegacyNpmMultilineArgumentWarning\n }",
"foreach ($ShimName in @('uloop.exe', 'uloop.cmd', 'uloop.ps1', 'uloop'))",
"if (Test-LegacyNpmUloopPath -CommandPath $ResolvedPath) {\n Write-LegacyNpmMultilineArgumentWarning\n }",
"Legacy npm shims can alter multiline PowerShell arguments before the native CLI receives them.",
} {
if !strings.Contains(setupScript, expected) {
Expand All @@ -60,8 +65,17 @@ func TestCommandForWindowsConfiguresUserPathAndLegacyCleanup(t *testing.T) {
if count := strings.Count(setupScript, "Legacy npm shims can alter multiline PowerShell arguments before the native CLI receives them."); count != 1 {
t.Fatalf("legacy npm multiline warning should have one message definition, got %d", count)
}
if count := strings.Count(setupScript, "Write-LegacyNpmMultilineArgumentWarning"); count != 3 {
t.Fatalf("legacy npm multiline warning should be centralized and called from two sites, got %d occurrences", count)
if count := strings.Count(setupScript, "Write-LegacyNpmMultilineArgumentWarning"); count != 2 {
t.Fatalf("legacy npm multiline warning should be centralized and called from one site, got %d occurrences", count)
}
if strings.Contains(setupScript, "Failed to remove the legacy npm uloop-cli package.") {
t.Fatal("legacy npm cleanup failure should not fail installation")
}
if strings.Contains(setupScript, "Get-Command uloop -ErrorAction SilentlyContinue") {
t.Fatal("Windows path shadowing should inspect persisted PATH instead of the current process PATH")
}
if strings.Contains(setupScript, "$RemovedAll") {
t.Fatal("legacy npm cleanup aggregate result should not be tracked when it is intentionally ignored")
}
}

Expand Down Expand Up @@ -511,8 +525,8 @@ func TestPosixInstallScriptRemovesAbsoluteLegacyNpmShimBeforePrependingPath(t *t
}
}

func TestPosixInstallScriptReportsManualRemovalWhenLegacyShimRemains(t *testing.T) {
// Verifies macOS legacy cleanup does not report success when npm leaves the old shim behind.
func TestPosixInstallScriptSilencesLegacyFailureWhenLegacyShimRemains(t *testing.T) {
// Verifies macOS legacy cleanup stays quiet when npm leaves the old shim behind.
if runtime.GOOS == "windows" {
t.Skip("POSIX shell setup is not available on Windows")
}
Expand Down Expand Up @@ -561,11 +575,11 @@ func TestPosixInstallScriptReportsManualRemovalWhenLegacyShimRemains(t *testing.
if strings.Contains(outputText, "Removed legacy npm package: uloop-cli") {
t.Fatalf("cleanup should not report success while the legacy shim remains:\n%s", outputText)
}
if !strings.Contains(outputText, "Could not remove the legacy npm package automatically.") {
t.Fatalf("cleanup should print manual removal guidance:\n%s", outputText)
if strings.Contains(outputText, "Could not remove the legacy npm package automatically.") {
t.Fatalf("cleanup should not report legacy npm removal failure:\n%s", outputText)
}
if !strings.Contains(outputText, "npm uninstall -g --prefix \""+legacyPrefix+"\" uloop-cli") {
t.Fatalf("manual removal guidance should include the inferred prefix:\n%s", outputText)
if strings.Contains(outputText, "npm uninstall -g --prefix \""+legacyPrefix+"\" uloop-cli") {
t.Fatalf("cleanup should not print manual removal guidance:\n%s", outputText)
}
if _, err := os.Stat(legacyUloop); err != nil {
t.Fatalf("legacy uloop should remain for this scenario: %v", err)
Expand Down
19 changes: 0 additions & 19 deletions cli/internal/install/posix_install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,22 +227,6 @@ is_legacy_npm_uloop_path() {
return 1
}

print_legacy_npm_manual_removal() {
legacy_uloop=$1
legacy_prefix=$2
echo "Could not remove the legacy npm package automatically."
if [ -n "$legacy_uloop" ]; then
echo "Legacy uloop command: $legacy_uloop"
fi
if [ -n "$legacy_prefix" ]; then
echo "Run this manually if that command still shadows the native CLI:"
echo " npm uninstall -g --prefix \"$legacy_prefix\" uloop-cli"
return
fi
echo "Run this manually if the old npm command still shadows the native CLI:"
echo " npm uninstall -g uloop-cli"
}

try_remove_legacy_npm_package() {
legacy_uloop=$1
legacy_prefix=""
Expand All @@ -251,11 +235,9 @@ try_remove_legacy_npm_package() {
fi
legacy_prefix=$(infer_npm_prefix_from_uloop_path "$legacy_uloop")
if [ -z "$legacy_prefix" ]; then
print_legacy_npm_manual_removal "$legacy_uloop" ""
return
fi
if ! command -v npm >/dev/null 2>&1; then
print_legacy_npm_manual_removal "$legacy_uloop" "$legacy_prefix"
return
fi
if npm uninstall -g --prefix "$legacy_prefix" uloop-cli; then
Expand All @@ -264,7 +246,6 @@ try_remove_legacy_npm_package() {
return
fi
fi
print_legacy_npm_manual_removal "$legacy_uloop" "$legacy_prefix"
}

try_remove_default_legacy_npm_package() {
Expand Down
71 changes: 12 additions & 59 deletions scripts/install.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,27 +202,6 @@ function Test-LegacyNpmUloopPath {
return $CommandContent.Contains("node_modules/uloop-cli") -or $CommandContent.Contains("node_modules\uloop-cli")
}

function Write-LegacyNpmManualRemoval {
param(
[string]$LegacyUloopPath,
[string]$LegacyPrefix
)

Write-Host "Could not remove the legacy npm package automatically."
if ($LegacyUloopPath) {
Write-Host "Legacy uloop command: $LegacyUloopPath"
}

if ($LegacyPrefix) {
Write-Host "Run this manually if that command still shadows the native CLI:"
Write-Host (' npm uninstall -g --prefix "' + $LegacyPrefix + '" uloop-cli')
return
}

Write-Host "Run this manually if the old npm command still shadows the native CLI:"
Write-Host " npm uninstall -g uloop-cli"
}

function Remove-LegacyNpmArtifacts {
param(
[string]$LegacyUloopPath,
Expand Down Expand Up @@ -303,7 +282,6 @@ function Invoke-LegacyNpmPackageRemoval {
}

if (-not $LegacyPrefix) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix
return $false
}

Expand All @@ -317,7 +295,6 @@ function Invoke-LegacyNpmPackageRemoval {
}

if (Test-LegacyNpmArtifactsExist -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix $LegacyPrefix
return $false
}

Expand Down Expand Up @@ -380,23 +357,11 @@ function Invoke-AllLegacyNpmPackageRemoval {
[string]$ExpectedUloopPath
)

$RemovedAll = $true
foreach ($LegacyUloopPath in (Get-LegacyNpmUloopPathsFromPath)) {
if (-not (Invoke-LegacyNpmPackageRemoval -LegacyUloopPath $LegacyUloopPath -ExpectedUloopPath $ExpectedUloopPath)) {
$RemovedAll = $false
}
Invoke-LegacyNpmPackageRemoval -LegacyUloopPath $LegacyUloopPath -ExpectedUloopPath $ExpectedUloopPath | Out-Null
}

Invoke-DefaultLegacyNpmPackageRemoval

foreach ($LegacyUloopPath in (Get-LegacyNpmUloopPathsFromPath)) {
if (-not [string]::Equals($LegacyUloopPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
Write-LegacyNpmManualRemoval -LegacyUloopPath $LegacyUloopPath -LegacyPrefix (Get-NpmPrefixFromUloopPath -CommandPath $LegacyUloopPath)
$RemovedAll = $false
}
}

return $RemovedAll
}

function Set-CurrentPathWithInstallDirectoryFirst {
Expand Down Expand Up @@ -446,12 +411,16 @@ function Get-FirstUloopCommandFromPath {
}

foreach ($PathEntry in ($PathValue -split ";")) {
if (-not $PathEntry) {
$NormalizedPathEntry = ConvertTo-NormalizedPath -Path $PathEntry
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
if (-not $NormalizedPathEntry) {
continue
}
if ($NormalizedPathEntry -match "^[A-Za-z]:$") {
$NormalizedPathEntry = $NormalizedPathEntry + "\"
}

foreach ($ShimName in @("uloop.exe", "uloop.cmd", "uloop.ps1", "uloop")) {
$CandidatePath = Join-Path $PathEntry $ShimName
$CandidatePath = Join-Path $NormalizedPathEntry $ShimName
if (Test-Path $CandidatePath -PathType Leaf) {
return $CandidatePath
}
Expand All @@ -461,38 +430,25 @@ function Get-FirstUloopCommandFromPath {
return $null
}

function Test-PersistedUloopCommandResolvesToExpectedPath {
function Report-PathShadowing {
param(
[string]$Directory,
[string]$ExpectedUloopPath
)

$MachinePath = [Environment]::GetEnvironmentVariable("Path", "Machine")
$UserPath = [Environment]::GetEnvironmentVariable("Path", "User")
$ResolvedPath = Get-FirstUloopCommandFromPath -PathValue ([string]::Join(";", @($MachinePath, $UserPath)))
if (-not $ResolvedPath) {
return $false
}

return [string]::Equals($ResolvedPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)
}

function Report-PathShadowing {
param(
[string]$Directory,
[string]$ExpectedUloopPath
)

$ResolvedCommand = Get-Command uloop -ErrorAction SilentlyContinue | Select-Object -First 1
if (-not $ResolvedCommand) {
return
}

if ([string]::Equals($ResolvedCommand.Source, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
if ([string]::Equals($ResolvedPath, $ExpectedUloopPath, [System.StringComparison]::OrdinalIgnoreCase)) {
return
}

Write-Host "Installed uloop to $ExpectedUloopPath, but PATH resolves uloop to:"
Write-Host " $($ResolvedCommand.Source)"
Write-Host " $ResolvedPath"
Write-Host "Move $Directory earlier in PATH, or remove the legacy installation if it owns that command."
}

Expand All @@ -506,10 +462,7 @@ function Invoke-CompatibilityWindowsInstall {

Write-Host "Configuring global uloop launcher..."
Set-UserPathWithInstallDirectoryFirst -Directory $Directory
$LegacyCleanupSucceeded = Invoke-AllLegacyNpmPackageRemoval -ExpectedUloopPath $ExpectedUloopPath
if ((-not $LegacyCleanupSucceeded) -and (-not (Test-PersistedUloopCommandResolvesToExpectedPath -ExpectedUloopPath $ExpectedUloopPath))) {
throw "Failed to remove the legacy npm uloop-cli package."
}
Invoke-AllLegacyNpmPackageRemoval -ExpectedUloopPath $ExpectedUloopPath | Out-Null

Report-PathShadowing -Directory $Directory -ExpectedUloopPath $ExpectedUloopPath
Write-Host "The package-owned User PATH entry was configured."
Expand Down
Loading
Loading