From b08e5476efe492f717570e024dd611e8bcda0ce0 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 00:14:04 +0200 Subject: [PATCH 01/12] fix(installer): expose stable Windows executable Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 ++ .../docs/getting-started/installation.md | 7 ++-- .../content/docs/reference/cli/self-update.md | 2 +- install.ps1 | 42 ++++++++++++++++++- .../.apm/skills/apm-usage/installation.md | 6 ++- scripts/windows/test-install-script.ps1 | 38 ++++++++++++++++- .../unit/test_windows_installer_launchers.py | 29 +++++++++++++ 7 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 tests/unit/test_windows_installer_launchers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d58656897..0ebb93acd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The Windows installer now exposes a version-stable `apm.exe` on `PATH`, so + Git Bash and bare process calls such as Python `subprocess.run(["apm", ...])` + resolve APM without relying on `cmd.exe` PATHEXT handling. (closes #2076) - `apm install host/org/repo/subpath#ref` on an unrecognised self-hosted FQDN no longer fails with a misleading "not accessible or doesn't exist" error; the failure reason now suggests setting `GITLAB_HOST` / `APM_GITLAB_HOSTS` diff --git a/docs/src/content/docs/getting-started/installation.md b/docs/src/content/docs/getting-started/installation.md index f42b46684..295a8def8 100644 --- a/docs/src/content/docs/getting-started/installation.md +++ b/docs/src/content/docs/getting-started/installation.md @@ -27,7 +27,7 @@ curl -sSL https://aka.ms/apm-unix | sh irm https://aka.ms/apm-windows | iex ``` -The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. +The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. On Windows, it exposes both `bin\apm.cmd` and a stable `current\apm.exe`, so Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` resolve the command. ### Installer options @@ -57,7 +57,8 @@ $env:VERSION = "v1.2.3"; irm https://aka.ms/apm-windows | iex # Saved script: pass -SkipChecksum only when the release has no .sha256 sidecar (not recommended). # .\install.ps1 v1.2.3 -SkipChecksum -# Custom directory for apm.cmd (default: %LOCALAPPDATA%\Programs\apm\bin) +# Custom directory for apm.cmd (default: %LOCALAPPDATA%\Programs\apm\bin). +# The installer also adds the sibling current directory containing apm.exe. $env:APM_INSTALL_DIR = "$env:LOCALAPPDATA\Programs\apm\bin"; irm https://aka.ms/apm-windows | iex # Fork, enterprise host, or internal mirror (GITHUB_URL must be https://) @@ -88,7 +89,7 @@ jobs: | Variable | Default | Description | |----------|---------|-------------| -| `APM_INSTALL_DIR` | `/usr/local/bin` (Unix) / `%LOCALAPPDATA%\Programs\apm\bin` (Windows) | Directory for the `apm` symlink / `apm.cmd` shim | +| `APM_INSTALL_DIR` | `/usr/local/bin` (Unix) / `%LOCALAPPDATA%\Programs\apm\bin` (Windows) | Directory for the Unix `apm` symlink or Windows `apm.cmd` shim. On Windows, the installer also adds the sibling `current` junction containing `apm.exe` to `PATH`. | | `APM_LIB_DIR` | `$(dirname APM_INSTALL_DIR)/lib/apm` | *(Unix only)* Directory for the full binary bundle. Must end with `/apm` (for example, `/lib/apm`). The installer rejects shared directories (e.g. `$HOME/.local/share`) to prevent accidental data loss. | | `GITHUB_URL` | `https://github.com` | Base GitHub URL (asset downloads **and** API host: `api.github.com` on github.com, `{GITHUB_URL}/api/v3` on GHES). Must be `https://` on Windows. | | `APM_REPO` | `microsoft/apm` | Repository as `owner/name` | diff --git a/docs/src/content/docs/reference/cli/self-update.md b/docs/src/content/docs/reference/cli/self-update.md index 712782770..dd35dd9c0 100644 --- a/docs/src/content/docs/reference/cli/self-update.md +++ b/docs/src/content/docs/reference/cli/self-update.md @@ -112,7 +112,7 @@ apm config unset self-update.install-dir ## Where the new binary lands -The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux, and a `%LOCALAPPDATA%\Programs\apm\bin\apm.cmd` shim pointing at the staged Windows release binary. Existing configuration under `~/.apm/` and your project files are untouched. +The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux. On Windows it writes `%LOCALAPPDATA%\Programs\apm\bin\apm.cmd` and updates the `%LOCALAPPDATA%\Programs\apm\current` junction containing `apm.exe`; both locations are added to `PATH`. Existing configuration under `~/.apm/` and your project files are untouched. ## After update diff --git a/install.ps1 b/install.ps1 index b84063206..ee0e458a3 100644 --- a/install.ps1 +++ b/install.ps1 @@ -7,7 +7,7 @@ # $env:VERSION = 'v1.2.3'; irm https://aka.ms/apm-windows | iex # .\install.ps1 v1.2.3 # -# Custom install location (directory that will contain apm.cmd): +# Custom install location (contains apm.cmd; sibling current contains apm.exe): # $env:APM_INSTALL_DIR = "$env:LOCALAPPDATA\Programs\apm\bin"; irm ... | iex # # Fork or private mirror: @@ -908,6 +908,44 @@ try { Remove-Item -Recurse -Force $backupDir -ErrorAction SilentlyContinue } + # Expose the complete onedir bundle through a version-stable junction. + # Putting this directory on PATH lets CreateProcess callers resolve the + # real apm.exe while keeping its sibling PyInstaller runtime files intact. + $currentDir = Join-Path $installRoot "current" + $currentExe = Join-Path $currentDir "apm.exe" + $newCurrentDir = "$currentDir.new-" + [System.Guid]::NewGuid().ToString("N") + $oldCurrentDir = $null + try { + New-Item -ItemType Junction -Path $newCurrentDir -Target $releaseDir | Out-Null + + if (Test-Path $currentDir) { + $currentItem = Get-Item -Force $currentDir + if (($currentItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -eq 0) { + throw "Refusing to replace non-junction path: $currentDir" + } + $oldCurrentDir = "$currentDir.old-" + [System.Guid]::NewGuid().ToString("N") + Move-Item -Path $currentDir -Destination $oldCurrentDir -Force + } + + Move-Item -Path $newCurrentDir -Destination $currentDir -Force + } catch { + Write-ErrorText "Failed to update stable executable path ${currentDir}: $_" + Remove-Item -Force $newCurrentDir -ErrorAction SilentlyContinue + if ($oldCurrentDir -and (Test-Path $oldCurrentDir) -and -not (Test-Path $currentDir)) { + Move-Item -Path $oldCurrentDir -Destination $currentDir -Force -ErrorAction SilentlyContinue + } + Write-ManualInstallHelp -GithubUrl $githubUrl -ApmRepo $apmRepo + exit 1 + } + if ($oldCurrentDir -and (Test-Path $oldCurrentDir)) { + Remove-Item -Force $oldCurrentDir -ErrorAction SilentlyContinue + } + if (-not (Test-Path $currentExe)) { + Write-ErrorText "Stable executable path is missing apm.exe: $currentExe" + Write-ManualInstallHelp -GithubUrl $githubUrl -ApmRepo $apmRepo + exit 1 + } + $shimPath = Join-Path $binDir "apm.cmd" # Prefer the literal %LOCALAPPDATA% token over the expanded profile path # so cmd.exe resolves the shim target at runtime. This avoids "The @@ -959,10 +997,12 @@ try { Set-Content -Path $shimPath -Value $shimContent -Encoding ASCII -NoNewline Add-ToUserPath -PathEntry $binDir + Add-ToUserPath -PathEntry $currentDir Write-Host "" Write-Success "APM $tagName installed successfully!" Write-Info "Command shim: $shimPath" + Write-Info "Stable executable: $currentExe" Write-Host "" Write-Info "Quick start:" Write-Host " apm init my-app # Create a new APM project" diff --git a/packages/apm-guide/.apm/skills/apm-usage/installation.md b/packages/apm-guide/.apm/skills/apm-usage/installation.md index 38f92e1d4..dcfa81da4 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/installation.md +++ b/packages/apm-guide/.apm/skills/apm-usage/installation.md @@ -58,7 +58,7 @@ Uses the same variables as `install.sh` where applicable (`GITHUB_URL`, `APM_REP # Pin a version (skips releases/latest API). Requires .sha256 on the release unless APM_SKIP_CHECKSUM=1 (emergency). $env:VERSION = "v1.2.3"; irm https://aka.ms/apm-windows | iex -# Custom shim directory (directory that will contain apm.cmd) +# Custom shim directory (contains apm.cmd; sibling current contains apm.exe) $env:APM_INSTALL_DIR = "$env:LOCALAPPDATA\Programs\apm\bin"; irm https://aka.ms/apm-windows | iex $env:GITHUB_URL = "https://github.corp.com" @@ -67,6 +67,10 @@ $env:VERSION = "v1.2.3" irm https://aka.ms/apm-windows | iex ``` +The Windows installer adds both the shim directory and the version-stable +`current` directory to `PATH`. The latter contains `apm.exe`, so Git Bash and +Python `subprocess.run(["apm", ...])` can resolve `apm` without `cmd.exe`. + ## Enterprise bootstrap mirrors Set `APM_INSTALLER_BASE_URL`, `APM_RELEASE_METADATA_URL`, `APM_RELEASE_BASE_URL`, `APM_PYPI_INDEX_URL`, and `APM_NO_DIRECT_FALLBACK=1` to install and update APM through an internal mirror while failing closed on public fallback. For verification, run the installer and `apm self-update --check` behind an egress proxy or wrappers that deny public GitHub, `aka.ms`, PyPI, Homebrew, and Scoop; only your mirror host should appear. The canonical setup, GHES scoping note, and full no-egress smoke recipe live in the [installation bootstrap mirror section](https://github.com/microsoft/apm/blob/main/docs/src/content/docs/getting-started/installation.md#enterprise-bootstrap-mirror-mode). diff --git a/scripts/windows/test-install-script.ps1 b/scripts/windows/test-install-script.ps1 index f16b24446..5b7047f4a 100644 --- a/scripts/windows/test-install-script.ps1 +++ b/scripts/windows/test-install-script.ps1 @@ -7,8 +7,8 @@ # %LOCALAPPDATA%\Programs\apm\releases\...), NOT from %TEMP%, so it # survives AppLocker / App Control for Business policies that block # executable launch from user-writable temp paths. -# 3. The shim written to APM_INSTALL_DIR points at the promoted release -# directory and the temp staging area is cleaned up. +# 3. The launchers point at the promoted release directory, including a +# stable apm.exe that bare CreateProcess callers can resolve. # 4. Upgrading over an existing install exercises the "move releaseDir # aside -> promote staging -> delete backup" path with no leftovers. # 5. The real `apm self-update` command launches install.ps1 successfully @@ -320,6 +320,40 @@ function Test-EndToEndInstall { Assert-True ($ver.Output -match $PinnedVersion.TrimStart("v")) "apm.cmd --version reports $PinnedVersion" } + $currentDir = Join-Path $prefix.Root "current" + $stableExe = Join-Path $currentDir "apm.exe" + Assert-True (Test-Path $stableExe) "Stable apm.exe is available at $stableExe" + + if (Test-Path $stableExe) { + $savedPath = $env:Path + try { + $env:Path = "$currentDir;$env:Path" + $pythonScript = @' +import subprocess +import sys + +result = subprocess.run(["apm", "--version"], capture_output=True, text=True) +print(result.stdout, end="") +print(result.stderr, end="", file=sys.stderr) +sys.exit(result.returncode) +'@ + $pythonOutput = & python -c $pythonScript 2>&1 + $pythonExit = $LASTEXITCODE + Assert-True ($pythonExit -eq 0) "Python subprocess resolves bare apm (got $pythonExit; output: $pythonOutput)" + Assert-True (($pythonOutput | Out-String) -match $PinnedVersion.TrimStart("v")) "Python subprocess reports $PinnedVersion" + + $bash = Get-Command bash -ErrorAction SilentlyContinue + if ($bash) { + $bashOutput = & bash -lc "command -v apm && apm --version" 2>&1 + $bashExit = $LASTEXITCODE + Assert-True ($bashExit -eq 0) "Git Bash resolves bare apm (got $bashExit; output: $bashOutput)" + Assert-True (($bashOutput | Out-String) -match $PinnedVersion.TrimStart("v")) "Git Bash reports $PinnedVersion" + } + } finally { + $env:Path = $savedPath + } + } + $leftover = Get-ChildItem -Path $prefix.TmpDir -Filter "apm-install-*" -Directory -ErrorAction SilentlyContinue Assert-True (-not $leftover) "No leftover apm-install-* directory in APM_TEMP_DIR" } finally { diff --git a/tests/unit/test_windows_installer_launchers.py b/tests/unit/test_windows_installer_launchers.py new file mode 100644 index 000000000..2c198f844 --- /dev/null +++ b/tests/unit/test_windows_installer_launchers.py @@ -0,0 +1,29 @@ +"""Regression coverage for Windows installer launcher resolution.""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] + + +def test_windows_installer_exposes_stable_executable_on_path() -> None: + """Bare CreateProcess callers must resolve apm.exe without PATHEXT.""" + installer = (ROOT / "install.ps1").read_text(encoding="utf-8") + + current_dir = '$currentDir = Join-Path $installRoot "current"' + current_exe = '$currentExe = Join-Path $currentDir "apm.exe"' + create_junction = "New-Item -ItemType Junction" + add_current_to_path = "Add-ToUserPath -PathEntry $currentDir" + + assert current_dir in installer + assert current_exe in installer + assert create_junction in installer + assert add_current_to_path in installer + assert installer.index(create_junction) < installer.index(add_current_to_path) + + +def test_windows_installer_e2e_covers_bare_subprocess_resolution() -> None: + """The Windows release gate must exercise the reporter's failing call.""" + test_script = (ROOT / "scripts/windows/test-install-script.ps1").read_text(encoding="utf-8") + + assert 'subprocess.run(["apm", "--version"]' in test_script + assert 'bash -lc "command -v apm && apm --version"' in test_script From ade13054afde0bba113aa66a81a28513ad55c905 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 04:06:46 +0200 Subject: [PATCH 02/12] test(installer): cover Windows launcher path edges Exercise real launcher resolution from paths containing spaces and cmd metacharacters, and fold review feedback on literal version checks, collision guidance, documentation, and changelog traceability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- .../docs/getting-started/installation.md | 4 +++- install.ps1 | 4 +++- scripts/windows/test-install-script.ps1 | 22 ++++++++++++++----- .../unit/test_windows_installer_launchers.py | 9 ++++++-- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ebb93acd..3d1230403 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The Windows installer now exposes a version-stable `apm.exe` on `PATH`, so Git Bash and bare process calls such as Python `subprocess.run(["apm", ...])` - resolve APM without relying on `cmd.exe` PATHEXT handling. (closes #2076) + resolve APM without relying on `cmd.exe` PATHEXT handling. (closes #2076, #2094) - `apm install host/org/repo/subpath#ref` on an unrecognised self-hosted FQDN no longer fails with a misleading "not accessible or doesn't exist" error; the failure reason now suggests setting `GITLAB_HOST` / `APM_GITLAB_HOSTS` diff --git a/docs/src/content/docs/getting-started/installation.md b/docs/src/content/docs/getting-started/installation.md index 295a8def8..de03b653c 100644 --- a/docs/src/content/docs/getting-started/installation.md +++ b/docs/src/content/docs/getting-started/installation.md @@ -27,7 +27,9 @@ curl -sSL https://aka.ms/apm-unix | sh irm https://aka.ms/apm-windows | iex ``` -The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. On Windows, it exposes both `bin\apm.cmd` and a stable `current\apm.exe`, so Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` resolve the command. +The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. + +On Windows, both `bin\apm.cmd` and a stable `current\apm.exe` are available. This lets Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` resolve the command. ### Installer options diff --git a/install.ps1 b/install.ps1 index ee0e458a3..13a6b5a59 100644 --- a/install.ps1 +++ b/install.ps1 @@ -921,7 +921,7 @@ try { if (Test-Path $currentDir) { $currentItem = Get-Item -Force $currentDir if (($currentItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -eq 0) { - throw "Refusing to replace non-junction path: $currentDir" + throw "Refusing to replace non-junction path: $currentDir. Move or remove it if safe, then rerun the installer." } $oldCurrentDir = "$currentDir.old-" + [System.Guid]::NewGuid().ToString("N") Move-Item -Path $currentDir -Destination $oldCurrentDir -Force @@ -997,6 +997,8 @@ try { Set-Content -Path $shimPath -Value $shimContent -Encoding ASCII -NoNewline Add-ToUserPath -PathEntry $binDir + # The onedir bundle must stay intact beside apm.exe. Add its stable + # junction after bin so bare executable lookup wins where PATHEXT is absent. Add-ToUserPath -PathEntry $currentDir Write-Host "" diff --git a/scripts/windows/test-install-script.ps1 b/scripts/windows/test-install-script.ps1 index 5b7047f4a..a44a51d77 100644 --- a/scripts/windows/test-install-script.ps1 +++ b/scripts/windows/test-install-script.ps1 @@ -269,7 +269,8 @@ function Get-ShimVersion { } function New-IsolatedPrefix { - $root = Join-Path ([System.IO.Path]::GetTempPath()) ("apm-install-test-" + [System.Guid]::NewGuid().ToString("N")) + # Spaces and a cmd metacharacter exercise launcher/path quoting end to end. + $root = Join-Path ([System.IO.Path]::GetTempPath()) ("APM Install Test & Edge " + [System.Guid]::NewGuid().ToString("N")) $binDir = Join-Path $root "bin" $tmpDir = Join-Path $root "tmp" New-Item -ItemType Directory -Force -Path $binDir | Out-Null @@ -286,6 +287,7 @@ function Test-EndToEndInstall { $prefix = New-IsolatedPrefix try { + Assert-True ($prefix.Root -match [regex]::Escape(" & ")) "Test prefix exercises spaces and a cmd metacharacter" Write-Info "Running install.ps1 (VERSION=$PinnedVersion, APM_INSTALL_DIR=$($prefix.BinDir), APM_TEMP_DIR=$($prefix.TmpDir))" $exitCode = Invoke-InstallScript -Version $PinnedVersion -BinDir $prefix.BinDir -TmpDir $prefix.TmpDir Assert-True ($exitCode -eq 0) "install.ps1 exits 0 (got $exitCode)" @@ -329,27 +331,37 @@ function Test-EndToEndInstall { try { $env:Path = "$currentDir;$env:Path" $pythonScript = @' +import os import subprocess import sys -result = subprocess.run(["apm", "--version"], capture_output=True, text=True) +result = subprocess.run( + ["apm", "--version"], + capture_output=True, + cwd=os.environ["APM_LAUNCH_TEST_CWD"], + text=True, +) print(result.stdout, end="") print(result.stderr, end="", file=sys.stderr) sys.exit(result.returncode) '@ + $env:APM_LAUNCH_TEST_CWD = $prefix.Root $pythonOutput = & python -c $pythonScript 2>&1 $pythonExit = $LASTEXITCODE Assert-True ($pythonExit -eq 0) "Python subprocess resolves bare apm (got $pythonExit; output: $pythonOutput)" - Assert-True (($pythonOutput | Out-String) -match $PinnedVersion.TrimStart("v")) "Python subprocess reports $PinnedVersion" + Assert-True (($pythonOutput | Out-String) -match [regex]::Escape($PinnedVersion.TrimStart("v"))) "Python subprocess reports $PinnedVersion" $bash = Get-Command bash -ErrorAction SilentlyContinue if ($bash) { - $bashOutput = & bash -lc "command -v apm && apm --version" 2>&1 + $bashCurrentDir = (& $bash.Source -lc 'cygpath -u "$1"' bash $currentDir).Trim() + $bashTestDir = (& $bash.Source -lc 'cygpath -u "$1"' bash $prefix.Root).Trim() + $bashOutput = & $bash.Source -lc 'cd "$1" && PATH="$2:$PATH" && command -v apm && apm --version' bash $bashTestDir $bashCurrentDir 2>&1 $bashExit = $LASTEXITCODE Assert-True ($bashExit -eq 0) "Git Bash resolves bare apm (got $bashExit; output: $bashOutput)" - Assert-True (($bashOutput | Out-String) -match $PinnedVersion.TrimStart("v")) "Git Bash reports $PinnedVersion" + Assert-True (($bashOutput | Out-String) -match [regex]::Escape($PinnedVersion.TrimStart("v"))) "Git Bash reports $PinnedVersion" } } finally { + Remove-Item Env:APM_LAUNCH_TEST_CWD -ErrorAction SilentlyContinue $env:Path = $savedPath } } diff --git a/tests/unit/test_windows_installer_launchers.py b/tests/unit/test_windows_installer_launchers.py index 2c198f844..b8643d213 100644 --- a/tests/unit/test_windows_installer_launchers.py +++ b/tests/unit/test_windows_installer_launchers.py @@ -18,6 +18,7 @@ def test_windows_installer_exposes_stable_executable_on_path() -> None: assert current_exe in installer assert create_junction in installer assert add_current_to_path in installer + assert "Refusing to replace non-junction path" in installer assert installer.index(create_junction) < installer.index(add_current_to_path) @@ -25,5 +26,9 @@ def test_windows_installer_e2e_covers_bare_subprocess_resolution() -> None: """The Windows release gate must exercise the reporter's failing call.""" test_script = (ROOT / "scripts/windows/test-install-script.ps1").read_text(encoding="utf-8") - assert 'subprocess.run(["apm", "--version"]' in test_script - assert 'bash -lc "command -v apm && apm --version"' in test_script + assert '["apm", "--version"],' in test_script + # Spaces and a cmd metacharacter in the prefix defend launcher quoting. + assert "APM Install Test & Edge" in test_script + assert 'cwd=os.environ["APM_LAUNCH_TEST_CWD"]' in test_script + assert 'PATH="$2:$PATH"' in test_script + assert "command -v apm && apm --version" in test_script From 71217417e68f7128699c266af5eb0bd1c939c86c Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 04:12:17 +0200 Subject: [PATCH 03/12] ci: surface Windows installer evidence early Run the focused installer E2E before the broad Windows unit suite so unrelated platform failures cannot hide the launcher regression gate required for this fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-release.yml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index 254a1f962..da9ddc51d 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -69,6 +69,16 @@ jobs: - name: Install dependencies run: uv sync --extra dev --extra build + # Keep the installer gate ahead of the broad unit suite so an unrelated + # platform test failure cannot hide installer-specific Windows evidence. + - name: Test install.ps1 end-to-end (Windows) + if: matrix.platform == 'windows' + shell: pwsh + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/windows/test-install-script.ps1 + # Unit tests run on every push for fast platform-regression signal. # Smoke is intentionally NOT included here: it duplicates ci-integration.yml's # merge-time smoke gate and burns a real codex binary download per platform @@ -109,14 +119,6 @@ jobs: run: | uv run pwsh scripts/windows/build-binary.ps1 - - name: Test install.ps1 end-to-end (Windows) - if: matrix.platform == 'windows' - shell: pwsh - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/windows/test-install-script.ps1 - - name: Upload binary as workflow artifact uses: actions/upload-artifact@v4 with: From 8f7d6d4dc68cf1fb9ac10378d52dedee1649f293 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 04:19:15 +0200 Subject: [PATCH 04/12] fix(installer): remove stale Windows junction safely Windows PowerShell 5.1 prompts when Remove-Item targets a junction whose release bundle is non-empty. Delete the junction object through Directory.Delete so unattended upgrades and reinstalls complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- install.ps1 | 8 ++++++-- tests/unit/test_windows_installer_launchers.py | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/install.ps1 b/install.ps1 index 13a6b5a59..c3ce1b133 100644 --- a/install.ps1 +++ b/install.ps1 @@ -930,7 +930,9 @@ try { Move-Item -Path $newCurrentDir -Destination $currentDir -Force } catch { Write-ErrorText "Failed to update stable executable path ${currentDir}: $_" - Remove-Item -Force $newCurrentDir -ErrorAction SilentlyContinue + if (Test-Path $newCurrentDir) { + [System.IO.Directory]::Delete($newCurrentDir) + } if ($oldCurrentDir -and (Test-Path $oldCurrentDir) -and -not (Test-Path $currentDir)) { Move-Item -Path $oldCurrentDir -Destination $currentDir -Force -ErrorAction SilentlyContinue } @@ -938,7 +940,9 @@ try { exit 1 } if ($oldCurrentDir -and (Test-Path $oldCurrentDir)) { - Remove-Item -Force $oldCurrentDir -ErrorAction SilentlyContinue + # Directory.Delete removes only the junction. Windows PowerShell 5.1 + # Remove-Item prompts for its non-empty target in NonInteractive mode. + [System.IO.Directory]::Delete($oldCurrentDir) } if (-not (Test-Path $currentExe)) { Write-ErrorText "Stable executable path is missing apm.exe: $currentExe" diff --git a/tests/unit/test_windows_installer_launchers.py b/tests/unit/test_windows_installer_launchers.py index b8643d213..1b99860da 100644 --- a/tests/unit/test_windows_installer_launchers.py +++ b/tests/unit/test_windows_installer_launchers.py @@ -13,12 +13,15 @@ def test_windows_installer_exposes_stable_executable_on_path() -> None: current_exe = '$currentExe = Join-Path $currentDir "apm.exe"' create_junction = "New-Item -ItemType Junction" add_current_to_path = "Add-ToUserPath -PathEntry $currentDir" + remove_old_junction = "[System.IO.Directory]::Delete($oldCurrentDir)" assert current_dir in installer assert current_exe in installer assert create_junction in installer assert add_current_to_path in installer assert "Refusing to replace non-junction path" in installer + assert remove_old_junction in installer + assert "Remove-Item -Force $oldCurrentDir" not in installer assert installer.index(create_junction) < installer.index(add_current_to_path) From 6cbfa84e3051173cca56fa45b6464d1643290f53 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 10 Jul 2026 04:37:18 +0200 Subject: [PATCH 05/12] test(installer): add Windows launcher integration gate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/build-release.yml | 3 +- .../docs/contributing/integration-testing.md | 1 + pyproject.toml | 2 +- tests/integration/conftest.py | 5 +++ .../test_windows_installer_launchers.py | 37 +++++++++++++++++++ 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/integration/test_windows_installer_launchers.py diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml index da9ddc51d..93a6d6acd 100644 --- a/.github/workflows/build-release.yml +++ b/.github/workflows/build-release.yml @@ -75,9 +75,10 @@ jobs: if: matrix.platform == 'windows' shell: pwsh env: + APM_E2E_TESTS: "1" GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/windows/test-install-script.ps1 + uv run pytest tests/integration/test_windows_installer_launchers.py -q # Unit tests run on every push for fast platform-regression signal. # Smoke is intentionally NOT included here: it duplicates ci-integration.yml's diff --git a/docs/src/content/docs/contributing/integration-testing.md b/docs/src/content/docs/contributing/integration-testing.md index f09d80d5c..105e97f23 100644 --- a/docs/src/content/docs/contributing/integration-testing.md +++ b/docs/src/content/docs/contributing/integration-testing.md @@ -48,6 +48,7 @@ what the test family you want actually requires. | --- | --- | --- | | `requires_e2e_mode` | Opt-in for the heavyweight golden-scenario suite | `export APM_E2E_TESTS=1` | | `requires_network_integration` | Opt-in for tests that hit live registries | `export APM_RUN_INTEGRATION_TESTS=1` | +| `requires_windows` | A Windows-only process or filesystem boundary | Run on Windows | | `requires_inference` | Opt-in for tests that call inference APIs | `export APM_RUN_INFERENCE_TESTS=1` | | `requires_github_token` | A token usable against `github.com` / GitHub Models | `export GITHUB_APM_PAT=...` (or `GITHUB_TOKEN`) | | `requires_ado_pat` | Azure DevOps PAT for ADO host tests | `export ADO_APM_PAT=...` | diff --git a/pyproject.toml b/pyproject.toml index 71d83010e..80597e1f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -163,6 +163,7 @@ markers = [ "requires_ado_pat: requires ADO_APM_PAT", "requires_ado_bearer: requires az CLI logged in + APM_TEST_ADO_BEARER=1", "requires_network_integration: requires APM_RUN_INTEGRATION_TESTS=1", + "requires_windows: requires a Windows host", "requires_apm_binary: requires built apm binary on PATH (or APM_BINARY_PATH)", "requires_runtime_codex: requires codex runtime installed", "requires_runtime_copilot: requires GitHub Copilot CLI runtime installed", @@ -170,4 +171,3 @@ markers = [ "requires_inference: requires APM_RUN_INFERENCE_TESTS=1 + model API access", "req: binds a test to one or more OpenAPM req-XXX ids (spec-conformance suite)", ] - diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 87c4ca12b..2b5f74beb 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -195,6 +195,10 @@ def _is_inference_mode() -> bool: return os.environ.get("APM_RUN_INFERENCE_TESTS") == "1" +def _is_windows() -> bool: + return _platform.system() == "Windows" + + def _has_apm_binary() -> bool: return _resolve_apm_binary() is not None @@ -221,6 +225,7 @@ def _has_runtime(name: str) -> bool: _is_network_integration, "APM_RUN_INTEGRATION_TESTS=1 not set", ), + "requires_windows": (_is_windows, "Windows required"), "requires_apm_binary": ( _has_apm_binary, "apm binary not found on PATH (set APM_BINARY_PATH or build via scripts/build-binary.sh)", diff --git a/tests/integration/test_windows_installer_launchers.py b/tests/integration/test_windows_installer_launchers.py new file mode 100644 index 000000000..f621440ce --- /dev/null +++ b/tests/integration/test_windows_installer_launchers.py @@ -0,0 +1,37 @@ +"""End-to-end regression coverage for Windows installer launchers.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +pytestmark = [ + pytest.mark.requires_e2e_mode, + pytest.mark.requires_windows, +] + +ROOT = Path(__file__).resolve().parents[2] + + +def test_windows_installer_exposes_stable_executable() -> None: + """Install APM and launch bare ``apm`` through Windows process boundaries.""" + result = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-ExecutionPolicy", + "Bypass", + "-File", + str(ROOT / "scripts" / "windows" / "test-install-script.ps1"), + ], + cwd=ROOT, + capture_output=True, + text=True, + timeout=600, + check=False, + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.rstrip().endswith("All install.ps1 integration tests passed.") From e951547b6a8d9501b0b1f9b3e0e08babb56a45b1 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sat, 11 Jul 2026 20:32:04 +0200 Subject: [PATCH 06/12] docs(changelog): record Windows apm.exe resolution fix The PR shipped the stable current\apm.exe junction and PATH wiring but never recorded a CHANGELOG entry, and the entry the reviewer saw earlier was lost across merges. Add an Unreleased Fixed entry following the project convention (trailing PR number, closes-issue inline) so the merged change is documented. Addresses Copilot review on CHANGELOG.md (PR-number convention) and the fold-vs-defer must-fold changelog rule. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b81a4494-6c7d-41ae-ab05-0762d70279af --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 674411a76..f4459d9f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --target intellij` now configures JetBrains Copilot MCP support while routing package file primitives through the Copilot profile. (by @sergio-sisternes-epam; closes #1957) (#2041) +- The Windows installer now exposes a version-stable `current\apm.exe` on + `PATH` (via a junction to the active release bundle) alongside the existing + `bin\apm.cmd` shim, so Git Bash and Python `subprocess.run(["apm", ...])` + resolve `apm` without `cmd.exe` PATHEXT expansion. (closes #2076) (#2094) ### Fixed From 56cdbe0e466fa6032715ac2e88e826491aca4127 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sat, 11 Jul 2026 20:45:24 +0200 Subject: [PATCH 07/12] fix(installer): fold panel findings on Windows junction path Fold the apm-review-panel recommendations that fall inside this PR's stated scope (raising the quality bar on the new junction surface): - Upgrade + reinstall E2E gates now verify current\apm.exe resolves and reports the new version after the junction swap, and assert no current.new-*/current.old-* temps leak at the install root. A wrong-target junction survives the installer's own Test-Path guard, so file existence alone was insufficient (converged: python-architect, test-coverage, supply-chain). - Guard the rollback Directory.Delete of the temp junction so a .NET exception cannot skip the old-junction restore (python-architect). - Drop the duplicated path from the non-junction throw; the catch wrapper already embeds it (cli-logging). - Correct the misleading "after bin" PATH comment: Add-ToUserPath prepends, so the junction precedes bin (devx-ux). - Add a static unit trap locking in the new junction-resolution E2E coverage; mutation-break verified. - Add from __future__ import annotations to the unit trap; active-voice the Windows installation sentence; consolidate the duplicate [Unreleased] Fixed heading (python-architect, doc-writer). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b81a4494-6c7d-41ae-ab05-0762d70279af --- CHANGELOG.md | 3 --- .../docs/getting-started/installation.md | 2 +- install.ps1 | 8 +++--- scripts/windows/test-install-script.ps1 | 27 +++++++++++++++++++ .../unit/test_windows_installer_launchers.py | 15 +++++++++++ 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4459d9f9..fc1c4a65b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,9 +22,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `PATH` (via a junction to the active release bundle) alongside the existing `bin\apm.cmd` shim, so Git Bash and Python `subprocess.run(["apm", ...])` resolve `apm` without `cmd.exe` PATHEXT expansion. (closes #2076) (#2094) - -### Fixed - - Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) diff --git a/docs/src/content/docs/getting-started/installation.md b/docs/src/content/docs/getting-started/installation.md index de03b653c..dccc4bae7 100644 --- a/docs/src/content/docs/getting-started/installation.md +++ b/docs/src/content/docs/getting-started/installation.md @@ -29,7 +29,7 @@ irm https://aka.ms/apm-windows | iex The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. -On Windows, both `bin\apm.cmd` and a stable `current\apm.exe` are available. This lets Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` resolve the command. +On Windows, the installer provides both `bin\apm.cmd` and a stable `current\apm.exe`, so Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` can resolve the command. ### Installer options diff --git a/install.ps1 b/install.ps1 index c3ce1b133..06d90c64b 100644 --- a/install.ps1 +++ b/install.ps1 @@ -921,7 +921,7 @@ try { if (Test-Path $currentDir) { $currentItem = Get-Item -Force $currentDir if (($currentItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -eq 0) { - throw "Refusing to replace non-junction path: $currentDir. Move or remove it if safe, then rerun the installer." + throw "Refusing to replace non-junction path. Move or remove it if safe, then rerun the installer." } $oldCurrentDir = "$currentDir.old-" + [System.Guid]::NewGuid().ToString("N") Move-Item -Path $currentDir -Destination $oldCurrentDir -Force @@ -931,7 +931,7 @@ try { } catch { Write-ErrorText "Failed to update stable executable path ${currentDir}: $_" if (Test-Path $newCurrentDir) { - [System.IO.Directory]::Delete($newCurrentDir) + try { [System.IO.Directory]::Delete($newCurrentDir) } catch { Write-ErrorText "Could not remove temp junction ${newCurrentDir}: $_" } } if ($oldCurrentDir -and (Test-Path $oldCurrentDir) -and -not (Test-Path $currentDir)) { Move-Item -Path $oldCurrentDir -Destination $currentDir -Force -ErrorAction SilentlyContinue @@ -1002,7 +1002,9 @@ try { Add-ToUserPath -PathEntry $binDir # The onedir bundle must stay intact beside apm.exe. Add its stable - # junction after bin so bare executable lookup wins where PATHEXT is absent. + # junction so bare executable lookup finds apm.exe where PATHEXT is absent + # (CreateProcess, Git Bash). Add-ToUserPath prepends, so this entry + # precedes bin in PATH. Add-ToUserPath -PathEntry $currentDir Write-Host "" diff --git a/scripts/windows/test-install-script.ps1 b/scripts/windows/test-install-script.ps1 index a44a51d77..6aececdea 100644 --- a/scripts/windows/test-install-script.ps1 +++ b/scripts/windows/test-install-script.ps1 @@ -402,6 +402,20 @@ function Test-CrossVersionUpgrade { $shimText = Get-Content $shim -Raw Assert-True ($shimText -match [regex]::Escape($PinnedVersion)) "Step 2: shim references $PinnedVersion path" + # The version-stable junction must re-point at the new release so bare + # CreateProcess / Git Bash callers resolve the upgraded apm.exe, not the + # previous release still on disk. A wrong-target junction survives the + # installer's own Test-Path guard, so assert the resolved version too. + $stableExe = Join-Path $prefix.Root "current\apm.exe" + Assert-True (Test-Path $stableExe) "Stable current\apm.exe exists after upgrade" + if (Test-Path $stableExe) { + $stableVer = & $stableExe --version 2>&1 + Assert-True (($stableVer | Out-String) -match [regex]::Escape($PinnedVersion.TrimStart("v"))) "current\apm.exe reports $PinnedVersion after upgrade" + } + $junctionLeftover = Get-ChildItem -Path $prefix.Root -Directory -ErrorAction SilentlyContinue | + Where-Object { $_.Name -like "current.new-*" -or $_.Name -like "current.old-*" } + Assert-True (-not $junctionLeftover) "No leftover current.new-* / current.old-* junction temps at install root" + # Both release dirs may coexist (we only replace the matching tag), # but the staging/backup helper dirs from the second install MUST be # cleaned up. @@ -453,6 +467,19 @@ function Test-SameVersionReinstall { Assert-True ($ver.ExitCode -eq 0) "apm.cmd --version exits 0 after reinstall (got $($ver.ExitCode))" Assert-True ($ver.Output -match $PinnedVersion.TrimStart("v")) "apm.cmd --version reports $PinnedVersion after reinstall" + # The junction is rebuilt on every install; after a same-version + # reinstall it must still resolve the freshly promoted apm.exe for bare + # CreateProcess / Git Bash callers. + $stableExe = Join-Path $prefix.Root "current\apm.exe" + Assert-True (Test-Path $stableExe) "Stable current\apm.exe exists after reinstall" + if (Test-Path $stableExe) { + $stableVer = & $stableExe --version 2>&1 + Assert-True (($stableVer | Out-String) -match [regex]::Escape($PinnedVersion.TrimStart("v"))) "current\apm.exe reports $PinnedVersion after reinstall" + } + $junctionLeftover = Get-ChildItem -Path $prefix.Root -Directory -ErrorAction SilentlyContinue | + Where-Object { $_.Name -like "current.new-*" -or $_.Name -like "current.old-*" } + Assert-True (-not $junctionLeftover) "No leftover current.new-* / current.old-* junction temps at install root after reinstall" + $leftoverStaging = Get-ChildItem -Path $releasesDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -like "*.new-*" -or $_.Name -like "*.old-*" } Assert-True (-not $leftoverStaging) "No leftover .new-* / .old-* dirs after reinstall (rollback path didn't trigger and backup was deleted)" diff --git a/tests/unit/test_windows_installer_launchers.py b/tests/unit/test_windows_installer_launchers.py index 1b99860da..9593b67fc 100644 --- a/tests/unit/test_windows_installer_launchers.py +++ b/tests/unit/test_windows_installer_launchers.py @@ -1,5 +1,7 @@ """Regression coverage for Windows installer launcher resolution.""" +from __future__ import annotations + from pathlib import Path ROOT = Path(__file__).resolve().parents[2] @@ -35,3 +37,16 @@ def test_windows_installer_e2e_covers_bare_subprocess_resolution() -> None: assert 'cwd=os.environ["APM_LAUNCH_TEST_CWD"]' in test_script assert 'PATH="$2:$PATH"' in test_script assert "command -v apm && apm --version" in test_script + + +def test_windows_e2e_verifies_junction_resolution_after_upgrade() -> None: + """Upgrade/reinstall gates must confirm the junction re-points at the new release.""" + test_script = (ROOT / "scripts/windows/test-install-script.ps1").read_text(encoding="utf-8") + + # A wrong-target junction survives the installer's own Test-Path guard, so + # the upgrade and reinstall gates must resolve current\apm.exe and assert + # its reported version, not merely that the file exists. + assert "$stableExe --version" in test_script + assert "junction temps at install root" in test_script + assert "current.new-*" in test_script + assert "current.old-*" in test_script From 42357f19e3f811cfcf67c2ef87bbe35f84cca8ca Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 17:38:58 +0200 Subject: [PATCH 08/12] test(installer): prove Windows junction safety Add behavioral collision preservation, native cmd resolution, and persisted PATH ordering checks so the Windows executable fix cannot regress its fail-closed or multi-shell promises. Keep installation guidance canonical across the doc surfaces. Addresses panel follow-ups from test-coverage-expert, python-architect, and doc-writer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../docs/getting-started/installation.md | 2 +- .../content/docs/reference/cli/self-update.md | 2 +- .../.apm/skills/apm-usage/installation.md | 6 +-- scripts/windows/test-install-script.ps1 | 48 ++++++++++++++++++- .../unit/test_windows_installer_launchers.py | 12 +++++ 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/docs/src/content/docs/getting-started/installation.md b/docs/src/content/docs/getting-started/installation.md index dccc4bae7..6d225295d 100644 --- a/docs/src/content/docs/getting-started/installation.md +++ b/docs/src/content/docs/getting-started/installation.md @@ -29,7 +29,7 @@ irm https://aka.ms/apm-windows | iex The installer automatically detects your platform (macOS/Linux/Windows, Intel/ARM), downloads the latest binary, and adds `apm` to your `PATH`. -On Windows, the installer provides both `bin\apm.cmd` and a stable `current\apm.exe`, so Git Bash and process APIs such as Python `subprocess.run(["apm", ...])` can resolve the command. +On Windows, the installer adds both `current` and `bin` to `PATH`, with the stable `current\apm.exe` first. Bare `apm` calls therefore resolve the real executable in native shells, Git Bash, and process APIs such as Python `subprocess.run(["apm", ...])`; `bin\apm.cmd` remains available for compatibility. ### Installer options diff --git a/docs/src/content/docs/reference/cli/self-update.md b/docs/src/content/docs/reference/cli/self-update.md index dd35dd9c0..06b57f39f 100644 --- a/docs/src/content/docs/reference/cli/self-update.md +++ b/docs/src/content/docs/reference/cli/self-update.md @@ -112,7 +112,7 @@ apm config unset self-update.install-dir ## Where the new binary lands -The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux. On Windows it writes `%LOCALAPPDATA%\Programs\apm\bin\apm.cmd` and updates the `%LOCALAPPDATA%\Programs\apm\current` junction containing `apm.exe`; both locations are added to `PATH`. Existing configuration under `~/.apm/` and your project files are untouched. +The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux. On Windows, self-update advances the stable executable path described in [Installation](../../getting-started/installation/). Existing configuration under `~/.apm/` and your project files are untouched. ## After update diff --git a/packages/apm-guide/.apm/skills/apm-usage/installation.md b/packages/apm-guide/.apm/skills/apm-usage/installation.md index dcfa81da4..3fe156022 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/installation.md +++ b/packages/apm-guide/.apm/skills/apm-usage/installation.md @@ -52,7 +52,7 @@ GITHUB_URL=https://github.corp.com VERSION=v1.2.3 sh install.sh ## Installer options (Windows PowerShell) -Uses the same variables as `install.sh` where applicable (`GITHUB_URL`, `APM_REPO`, `VERSION`, `APM_INSTALL_DIR`). See the full variable table, Actions example, and checksum rules in [installation.md](https://github.com/microsoft/apm/blob/main/docs/src/content/docs/getting-started/installation.md). +Uses the same variables as `install.sh` where applicable (`GITHUB_URL`, `APM_REPO`, `VERSION`, `APM_INSTALL_DIR`). See the full variable table, Actions example, checksum rules, and canonical Windows `PATH` layout in [installation.md](https://github.com/microsoft/apm/blob/main/docs/src/content/docs/getting-started/installation.md). ```powershell # Pin a version (skips releases/latest API). Requires .sha256 on the release unless APM_SKIP_CHECKSUM=1 (emergency). @@ -67,10 +67,6 @@ $env:VERSION = "v1.2.3" irm https://aka.ms/apm-windows | iex ``` -The Windows installer adds both the shim directory and the version-stable -`current` directory to `PATH`. The latter contains `apm.exe`, so Git Bash and -Python `subprocess.run(["apm", ...])` can resolve `apm` without `cmd.exe`. - ## Enterprise bootstrap mirrors Set `APM_INSTALLER_BASE_URL`, `APM_RELEASE_METADATA_URL`, `APM_RELEASE_BASE_URL`, `APM_PYPI_INDEX_URL`, and `APM_NO_DIRECT_FALLBACK=1` to install and update APM through an internal mirror while failing closed on public fallback. For verification, run the installer and `apm self-update --check` behind an egress proxy or wrappers that deny public GitHub, `aka.ms`, PyPI, Homebrew, and Scoop; only your mirror host should appear. The canonical setup, GHES scoping note, and full no-egress smoke recipe live in the [installation bootstrap mirror section](https://github.com/microsoft/apm/blob/main/docs/src/content/docs/getting-started/installation.md#enterprise-bootstrap-mirror-mode). diff --git a/scripts/windows/test-install-script.ps1 b/scripts/windows/test-install-script.ps1 index 6aececdea..45d144f94 100644 --- a/scripts/windows/test-install-script.ps1 +++ b/scripts/windows/test-install-script.ps1 @@ -115,7 +115,7 @@ function Test-MoveThenTestOrdering { if ($errors -and $errors.Count -gt 0) { return } # Find every Move-Item invocation that mentions $packageDir and $stagingDir - # (either as -Path/-Destination args or as positional values) — that's our + # (either as -Path/-Destination args or as positional values) -- that's our # staging move. $moveCalls = $ast.FindAll({ param($n) @@ -327,9 +327,26 @@ function Test-EndToEndInstall { Assert-True (Test-Path $stableExe) "Stable apm.exe is available at $stableExe" if (Test-Path $stableExe) { + $userPath = [Environment]::GetEnvironmentVariable("Path", "User") + $userPathEntries = @() + if ($userPath) { + $userPathEntries = $userPath.Split(";", [System.StringSplitOptions]::RemoveEmptyEntries) + } + $currentPathIndex = [Array]::IndexOf($userPathEntries, $currentDir) + $binPathIndex = [Array]::IndexOf($userPathEntries, $prefix.BinDir) + Assert-True ($currentPathIndex -ge 0) "Stable executable directory is present in user PATH" + Assert-True ($binPathIndex -ge 0) "Command shim directory is present in user PATH" + Assert-True (($currentPathIndex -ge 0) -and ($currentPathIndex -lt $binPathIndex)) "Stable executable directory precedes command shim directory in user PATH" + $savedPath = $env:Path try { $env:Path = "$currentDir;$env:Path" + $env:APM_LAUNCH_TEST_PATH = "$currentDir;$($prefix.BinDir);$savedPath" + $cmdOutput = & cmd.exe /d /c 'set "PATH=%APM_LAUNCH_TEST_PATH%" && apm --version' 2>&1 + $cmdExit = $LASTEXITCODE + Assert-True ($cmdExit -eq 0) "cmd.exe resolves bare apm (got $cmdExit; output: $cmdOutput)" + Assert-True (($cmdOutput | Out-String) -match [regex]::Escape($PinnedVersion.TrimStart("v"))) "cmd.exe reports $PinnedVersion" + $pythonScript = @' import os import subprocess @@ -362,6 +379,7 @@ sys.exit(result.returncode) } } finally { Remove-Item Env:APM_LAUNCH_TEST_CWD -ErrorAction SilentlyContinue + Remove-Item Env:APM_LAUNCH_TEST_PATH -ErrorAction SilentlyContinue $env:Path = $savedPath } } @@ -373,6 +391,33 @@ sys.exit(result.returncode) } } +# --------------------------------------------------------------------------- +# Test 3b: A real directory at current is user-owned data, not an installer +# junction. The installer must fail closed without deleting its contents. +# --------------------------------------------------------------------------- + +function Test-NonJunctionCollision { + Write-Step "Test 3b: non-junction current path is preserved" + + $prefix = New-IsolatedPrefix + try { + $currentDir = Join-Path $prefix.Root "current" + $canary = Join-Path $currentDir "user-data.txt" + New-Item -ItemType Directory -Force -Path $currentDir | Out-Null + Set-Content -Path $canary -Value "preserve me" -Encoding ASCII + + $exitCode = Invoke-InstallScript -Version $PinnedVersion -BinDir $prefix.BinDir -TmpDir $prefix.TmpDir + Assert-True ($exitCode -eq 1) "Installer refuses a non-junction current path (got $exitCode)" + Assert-True (Test-Path $canary -PathType Leaf) "Non-junction current path preserves its canary file" + + $junctionLeftover = Get-ChildItem -Path $prefix.Root -Directory -ErrorAction SilentlyContinue | + Where-Object { $_.Name -like "current.new-*" -or $_.Name -like "current.old-*" } + Assert-True (-not $junctionLeftover) "Collision failure leaves no current.new-* / current.old-* junction temps" + } finally { + Remove-Item -Recurse -Force $prefix.Root -ErrorAction SilentlyContinue + } +} + # --------------------------------------------------------------------------- # Test 4a: Cross-version upgrade. Install OlderVersion, then PinnedVersion, # into the same prefix. The shim must end up pointing at PinnedVersion's @@ -566,6 +611,7 @@ Test-Sha256Fallback Test-MoveThenTestOrdering Test-AntivirusDetector Test-EndToEndInstall +Test-NonJunctionCollision Test-CrossVersionUpgrade Test-SameVersionReinstall Test-SelfUpdateCommand diff --git a/tests/unit/test_windows_installer_launchers.py b/tests/unit/test_windows_installer_launchers.py index 9593b67fc..9ba8afda0 100644 --- a/tests/unit/test_windows_installer_launchers.py +++ b/tests/unit/test_windows_installer_launchers.py @@ -37,6 +37,18 @@ def test_windows_installer_e2e_covers_bare_subprocess_resolution() -> None: assert 'cwd=os.environ["APM_LAUNCH_TEST_CWD"]' in test_script assert 'PATH="$2:$PATH"' in test_script assert "command -v apm && apm --version" in test_script + assert "cmd.exe /d /c" in test_script + assert "Stable executable directory precedes command shim directory in user PATH" in test_script + + +def test_windows_installer_e2e_covers_non_junction_collision() -> None: + """The Windows release gate must prove collision failures preserve data.""" + test_script = (ROOT / "scripts/windows/test-install-script.ps1").read_text(encoding="utf-8") + + assert "function Test-NonJunctionCollision" in test_script + assert "Installer refuses a non-junction current path" in test_script + assert "Non-junction current path preserves its canary file" in test_script + assert "Test-NonJunctionCollision" in test_script[test_script.index("# Runner") :] def test_windows_e2e_verifies_junction_resolution_after_upgrade() -> None: From b5f835ac1a58e772d3905b9adf48f9a3312bf7b7 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 18:01:33 +0200 Subject: [PATCH 09/12] docs(installer): fix merge-time drift Repair the self-update link to the canonical Windows PATH guidance and remove stale changelog lines introduced while merging main. This preserves discoverability and contributor attribution. Addresses doc-writer panel follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 3 --- docs/src/content/docs/reference/cli/self-update.md | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e590624bb..9e6e9ab4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,9 +93,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `PATH` (via a junction to the active release bundle) alongside the existing `bin\apm.cmd` shim, so Git Bash and Python `subprocess.run(["apm", ...])` resolve `apm` without `cmd.exe` PATHEXT expansion. (closes #2076) (#2094) -- Azure DevOps marketplace checks now preserve suffix-free `/_git/` URLs - and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) - (by @sergio-sisternes-epam, closes #1957, #2041) ### Performance diff --git a/docs/src/content/docs/reference/cli/self-update.md b/docs/src/content/docs/reference/cli/self-update.md index 06b57f39f..b49a72348 100644 --- a/docs/src/content/docs/reference/cli/self-update.md +++ b/docs/src/content/docs/reference/cli/self-update.md @@ -112,7 +112,7 @@ apm config unset self-update.install-dir ## Where the new binary lands -The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux. On Windows, self-update advances the stable executable path described in [Installation](../../getting-started/installation/). Existing configuration under `~/.apm/` and your project files are untouched. +The installer writes to the same location the install script uses -- by default `/usr/local/bin/apm` on macOS/Linux. On Windows, self-update advances the stable executable path described in [Installation](../../../getting-started/installation/). Existing configuration under `~/.apm/` and your project files are untouched. ## After update From 137531ef75faa9e35363a4c7b49d08807f7707dd Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 22:45:17 +0200 Subject: [PATCH 10/12] test(windows): guard stable executable owner Add an explicit canonical-owner row for the Windows stable executable path (install.ps1's $currentDir / $currentExe) and a matching regression test plus a static AC8 boundary guard in lint-architecture-boundaries.sh, so no production Python, workflow YAML, or non-test Windows script can re-derive current/apm.exe. install.ps1 and scripts/windows/test-install-script.ps1 remain the sanctioned owner and black-box validator respectively. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7955c89b-a997-42aa-9c45-ef4c7fe4b1e7 --- .../instructions/architecture.instructions.md | 1 + scripts/lint-architecture-boundaries.sh | 36 ++++++++++++++++++ .../test_architecture_authorities.py | 37 +++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md index c30293ffc..c24212159 100644 --- a/.github/instructions/architecture.instructions.md +++ b/.github/instructions/architecture.instructions.md @@ -39,6 +39,7 @@ instead of re-deriving the answer locally. | Install success / failure outcome | the canonical install-outcome path | | Neutral hook shape -> per-target native | the neutral hook IR + per-target integrators | | File-level deploy / sync / cleanup | BaseIntegrator (see integrators.instructions.md) | +| Windows stable executable path | install.ps1 ($currentDir / $currentExe) | If you are about to compute one of these locally, stop and call the owner. If the owner is missing a case you need, EXTEND the owner -- diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh index 3df7eaf83..8dd12b208 100755 --- a/scripts/lint-architecture-boundaries.sh +++ b/scripts/lint-architecture-boundaries.sh @@ -195,6 +195,42 @@ if ! grep -A8 'def add_marketplace' src/apm_cli/marketplace/registry.py \ violations=$((violations + 1)) fi +echo "[*] AC8: Windows installer authorities" +windows_owner_ok=1 +if ! grep -qF '$currentDir = Join-Path $installRoot "current"' install.ps1; then + windows_owner_ok=0 +fi +if ! grep -qF '$currentExe = Join-Path $currentDir "apm.exe"' install.ps1; then + windows_owner_ok=0 +fi +if ! grep -qF 'Add-ToUserPath -PathEntry $currentDir' install.ps1; then + windows_owner_ok=0 +fi +windows_guarded_files=$( + { + find scripts/windows -maxdepth 1 -name '*.ps1' ! -name 'test-*.ps1' + find src/apm_cli -name '*.py' + find .github/workflows -maxdepth 1 \( -name '*.yml' -o -name '*.yaml' \) + } 2>/dev/null +) +windows_dup_hits=$( + grep -EnH \ + 'Join-Path[[:space:]]+\S+[[:space:]]+["'\'']current["'\'']|current[\\/]apm\.exe' \ + $windows_guarded_files 2>/dev/null \ + | grep -v 'architecture-authority-exempt:' \ + || true +) +if [ "$windows_owner_ok" -eq 0 ] || [ -n "$windows_dup_hits" ]; then + echo "[x] Windows stable executable path belongs to install.ps1" + if [ "$windows_owner_ok" -eq 0 ]; then + echo " install.ps1 is missing one or more canonical owner statements" + fi + if [ -n "$windows_dup_hits" ]; then + echo "$windows_dup_hits" + fi + violations=$((violations + 1)) +fi + if [ "$violations" -gt 0 ]; then echo "[x] $violations architecture boundary rule(s) failed" exit 1 diff --git a/tests/integration/test_architecture_authorities.py b/tests/integration/test_architecture_authorities.py index 853f4771c..196067f2a 100644 --- a/tests/integration/test_architecture_authorities.py +++ b/tests/integration/test_architecture_authorities.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re from dataclasses import replace from pathlib import Path @@ -201,6 +202,42 @@ def test_dependency_winner_selection_has_one_algorithm() -> None: assert duplicate not in source +def test_windows_stable_executable_path_has_one_canonical_owner() -> None: + """install.ps1 alone may define the stable current/apm.exe location.""" + root = Path(__file__).parents[2] + installer = (root / "install.ps1").read_text(encoding="utf-8") + guard = (root / "scripts/lint-architecture-boundaries.sh").read_text() + + assert '$currentDir = Join-Path $installRoot "current"' in installer + assert '$currentExe = Join-Path $currentDir "apm.exe"' in installer + assert "Add-ToUserPath -PathEntry $currentDir" in installer + assert "Windows stable executable path belongs to install.ps1" in guard + + join_path_current = re.compile(r'Join-Path\s+\S+\s+["\']current["\']') + literal_stable_exe = re.compile(r"current[\\/]apm\.exe") + + def derives_stable_path(text: str) -> bool: + return bool(join_path_current.search(text) or literal_stable_exe.search(text)) + + guarded_windows_scripts = [ + path + for path in (root / "scripts/windows").glob("*.ps1") + if not path.name.startswith("test-") + ] + guarded_python_files = list((root / "src/apm_cli").rglob("*.py")) + guarded_workflow_files = list((root / ".github/workflows").glob("*.yml")) + list( + (root / ".github/workflows").glob("*.yaml") + ) + + duplicate_owners = [ + path.relative_to(root).as_posix() + for path in (*guarded_windows_scripts, *guarded_python_files, *guarded_workflow_files) + if derives_stable_path(path.read_text(encoding="utf-8")) + ] + + assert duplicate_owners == [] + + def test_tls_injection_has_one_canonical_authority() -> None: """Only the parent TLS owner and standalone child bootstrap may inject.""" root = Path(__file__).parents[2] From 5c003c6a5c5478d238e2b16843858e0b4639bc4b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 23:01:48 +0200 Subject: [PATCH 11/12] test(windows): unify stable path boundary Replace the duplicated Windows stable-executable-path regex/glob scan in scripts/lint-architecture-boundaries.sh (AC8) and tests/integration/test_architecture_authorities.py with one canonical checker, scripts/check_windows_stable_path_owner.py. The two guards had already drifted: Bash honored line-level architecture-authority-exempt: markers but the Python test did not. The checker is stdlib-only, typed, and owns owner-statement presence in install.ps1 plus line-oriented duplicate detection (literal current\apm.exe / current/apm.exe and positional or named-parameter Join-Path "current" derivations) across src/apm_cli/**/*.py, .github/workflows/**/*.{yml,yaml}, and scripts/windows/**/*.ps1, excluding test-*.ps1 validators. Both consumers now call it instead of re-implementing its regexes, globs, or exemption handling. Added TDD unit tests under tests/unit/scripts/ covering owner presence/absence, positional and named (including reordered) Join-Path duplicates, literal path duplicates (both separators), the exemption marker, test-script exclusion, nested/extension-varied workflow discovery, and CLI exit code plus diagnostic content. Mutation-tested locally: a probe file deriving the path via a named-parameter Join-Path broke both the Bash lint and the architecture-authorities test; adding a line-level exemption made both pass again; the probe was then removed and the tree re-verified clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7955c89b-a997-42aa-9c45-ef4c7fe4b1e7 --- scripts/check_windows_stable_path_owner.py | 203 ++++++++++++++++ scripts/lint-architecture-boundaries.sh | 38 +-- .../test_architecture_authorities.py | 63 ++--- tests/unit/scripts/__init__.py | 1 + .../test_check_windows_stable_path_owner.py | 230 ++++++++++++++++++ 5 files changed, 476 insertions(+), 59 deletions(-) create mode 100644 scripts/check_windows_stable_path_owner.py create mode 100644 tests/unit/scripts/__init__.py create mode 100644 tests/unit/scripts/test_check_windows_stable_path_owner.py diff --git a/scripts/check_windows_stable_path_owner.py b/scripts/check_windows_stable_path_owner.py new file mode 100644 index 000000000..5eec87358 --- /dev/null +++ b/scripts/check_windows_stable_path_owner.py @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +"""Canonical checker for the Windows stable executable path owner. + +install.ps1 is the single canonical owner of the Windows "stable" +executable path: it alone may declare ``$currentDir`` (the +``current`` directory under the install root), ``$currentExe`` (the +``apm.exe`` file inside it), and the ``Add-ToUserPath`` call that +publishes ``$currentDir`` on the user's PATH. No other production +source may re-derive that path, or the two copies will silently drift +the next time one side is patched (see +``.github/instructions/architecture.instructions.md``). + +This module is the ONE place that owns this check. It is consumed by +both: + + * ``scripts/lint-architecture-boundaries.sh`` (AC8), which shells out + to this script and folds any nonzero exit into a single boundary + violation; and + * ``tests/integration/test_architecture_authorities.py``, which + imports and calls this module directly instead of re-implementing + its regexes or globs. + +Two things are checked: + +1. **Owner presence** -- ``install.ps1`` must contain each of the + required owner statements verbatim. +2. **Duplicate derivation** -- no production file under + ``src/apm_cli/**/*.py``, ``.github/workflows/**/*.{yml,yaml}``, or + ``scripts/windows/**/*.ps1`` (excluding files whose basename starts + with ``test-``, which are black-box validators, not owners) may + contain a literal ``current\\apm.exe`` / ``current/apm.exe`` path or + a ``Join-Path`` expression that derives a quoted ``current`` child, + in either positional (``Join-Path $x "current"``) or named-parameter + (``Join-Path -Path $x -ChildPath "current"``) form. + +A line that carries the ``architecture-authority-exempt:`` marker is +always skipped, regardless of which pattern it would otherwise match. +This is a narrow, line-oriented text scan -- it is not a PowerShell (or +Python, or YAML) parser, and it is not meant to become one. + +Exit code is 0 when clean, 1 when any violation is found. Diagnostics +are printed one per line, in deterministic (sorted) order. +""" + +from __future__ import annotations + +import argparse +import re +import sys +from dataclasses import dataclass +from pathlib import Path + +# Repository root, derived from this file's location (scripts/). +REPO_ROOT = Path(__file__).resolve().parent.parent + +INSTALL_PS1_RELATIVE = "install.ps1" + +# Verbatim statements that must exist somewhere in install.ps1. These are +# intentionally exact strings, not regexes: the owner statements are +# hand-written and any drift in their shape should fail loudly rather than +# be silently tolerated by a looser pattern. +REQUIRED_OWNER_STATEMENTS: tuple[str, ...] = ( + '$currentDir = Join-Path $installRoot "current"', + '$currentExe = Join-Path $currentDir "apm.exe"', + "Add-ToUserPath -PathEntry $currentDir", +) + +# Line-level opt-out marker. Any line containing this text is skipped by +# the duplicate scan entirely, matching the convention documented at the +# top of scripts/lint-architecture-boundaries.sh. +EXEMPT_MARKER = "architecture-authority-exempt:" + +# Literal stable-path forms: current\apm.exe (Windows) or current/apm.exe. +_LITERAL_STABLE_EXE = re.compile(r"current[\\/]apm\.exe") + +# A Join-Path call is a duplicate derivation if it appears on the same +# line as a quoted "current" (single or double quotes), regardless of +# whether "current" is passed positionally or via -ChildPath / -Path. +_JOIN_PATH_CALL = re.compile(r"Join-Path", re.IGNORECASE) +_QUOTED_CURRENT = re.compile(r"""['"]current['"]""") + +# (subdirectory relative to repo root, file suffixes to scan) +_GUARDED_LOCATIONS: tuple[tuple[str, tuple[str, ...]], ...] = ( + ("src/apm_cli", (".py",)), + (".github/workflows", (".yml", ".yaml")), + ("scripts/windows", (".ps1",)), +) + + +@dataclass(frozen=True) +class DuplicateHit: + """One line that re-derives the canonical stable path.""" + + path: Path + line_no: int + text: str + + +def _is_duplicate_line(line: str) -> bool: + """Return True if `line` re-derives the stable current/apm.exe path.""" + if EXEMPT_MARKER in line: + return False + if _LITERAL_STABLE_EXE.search(line): + return True + return bool(_JOIN_PATH_CALL.search(line) and _QUOTED_CURRENT.search(line)) + + +def _guarded_files(root: Path) -> list[Path]: + """List production files subject to the duplicate-derivation scan. + + Recurses under each guarded location and excludes any file whose + basename starts with ``test-`` (black-box validators such as + ``scripts/windows/test-install-script.ps1`` are not owners). + """ + files: list[Path] = [] + for subdir, suffixes in _GUARDED_LOCATIONS: + base = root / subdir + if not base.is_dir(): + continue + for path in base.rglob("*"): + if not path.is_file(): + continue + if path.suffix not in suffixes: + continue + if path.name.startswith("test-"): + continue + files.append(path) + return sorted(files) + + +def find_owner_violations(root: Path) -> list[str]: + """Return diagnostics for missing canonical owner statements.""" + install_ps1 = root / INSTALL_PS1_RELATIVE + if not install_ps1.is_file(): + return [f"[x] {INSTALL_PS1_RELATIVE} does not exist; cannot own the stable path"] + text = install_ps1.read_text(encoding="utf-8") + return [ + f"[x] install.ps1 is missing canonical owner statement: {statement}" + for statement in REQUIRED_OWNER_STATEMENTS + if statement not in text + ] + + +def find_duplicate_hits(root: Path) -> list[DuplicateHit]: + """Return every duplicate-derivation line found in guarded files.""" + hits: list[DuplicateHit] = [] + for path in _guarded_files(root): + try: + text = path.read_text(encoding="utf-8") + except (UnicodeDecodeError, OSError): + continue + for line_no, line in enumerate(text.splitlines(), start=1): + if _is_duplicate_line(line): + hits.append(DuplicateHit(path=path, line_no=line_no, text=line.strip())) + return hits + + +def find_duplicate_violations(root: Path) -> list[str]: + """Return diagnostics, one per duplicate-derivation line, sorted.""" + diagnostics = [ + f"[x] duplicate stable-path derivation: {hit.path.relative_to(root).as_posix()}:" + f"{hit.line_no}: {hit.text}" + for hit in find_duplicate_hits(root) + ] + return sorted(diagnostics) + + +def check(root: Path) -> list[str]: + """Return all diagnostics for `root`. Empty list means clean.""" + return [*find_owner_violations(root), *find_duplicate_violations(root)] + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + description=( + "Check that install.ps1 is the sole owner of the Windows stable " + "executable path (current/apm.exe)." + ) + ) + parser.add_argument( + "--root", + type=Path, + default=None, + help="Repository root to scan (defaults to the real repository root).", + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + root = args.root.resolve() if args.root is not None else REPO_ROOT + violations = check(root) + for diagnostic in violations: + print(diagnostic) + if violations: + print(f"[x] {len(violations)} Windows stable path owner violation(s) found") + return 1 + print("[+] Windows stable executable path owner check clean") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/lint-architecture-boundaries.sh b/scripts/lint-architecture-boundaries.sh index 8dd12b208..021236495 100755 --- a/scripts/lint-architecture-boundaries.sh +++ b/scripts/lint-architecture-boundaries.sh @@ -196,38 +196,14 @@ if ! grep -A8 'def add_marketplace' src/apm_cli/marketplace/registry.py \ fi echo "[*] AC8: Windows installer authorities" -windows_owner_ok=1 -if ! grep -qF '$currentDir = Join-Path $installRoot "current"' install.ps1; then - windows_owner_ok=0 -fi -if ! grep -qF '$currentExe = Join-Path $currentDir "apm.exe"' install.ps1; then - windows_owner_ok=0 -fi -if ! grep -qF 'Add-ToUserPath -PathEntry $currentDir' install.ps1; then - windows_owner_ok=0 -fi -windows_guarded_files=$( - { - find scripts/windows -maxdepth 1 -name '*.ps1' ! -name 'test-*.ps1' - find src/apm_cli -name '*.py' - find .github/workflows -maxdepth 1 \( -name '*.yml' -o -name '*.yaml' \) - } 2>/dev/null -) -windows_dup_hits=$( - grep -EnH \ - 'Join-Path[[:space:]]+\S+[[:space:]]+["'\'']current["'\'']|current[\\/]apm\.exe' \ - $windows_guarded_files 2>/dev/null \ - | grep -v 'architecture-authority-exempt:' \ - || true -) -if [ "$windows_owner_ok" -eq 0 ] || [ -n "$windows_dup_hits" ]; then +# Owner presence + duplicate-derivation scanning both live in the single +# canonical checker so this guard and the architecture test suite cannot +# drift apart. See scripts/check_windows_stable_path_owner.py. +windows_owner_output=$(python3 scripts/check_windows_stable_path_owner.py --root "$ROOT" 2>&1) +windows_owner_status=$? +if [ "$windows_owner_status" -ne 0 ]; then echo "[x] Windows stable executable path belongs to install.ps1" - if [ "$windows_owner_ok" -eq 0 ]; then - echo " install.ps1 is missing one or more canonical owner statements" - fi - if [ -n "$windows_dup_hits" ]; then - echo "$windows_dup_hits" - fi + echo "$windows_owner_output" violations=$((violations + 1)) fi diff --git a/tests/integration/test_architecture_authorities.py b/tests/integration/test_architecture_authorities.py index 196067f2a..90503e8d4 100644 --- a/tests/integration/test_architecture_authorities.py +++ b/tests/integration/test_architecture_authorities.py @@ -2,13 +2,34 @@ from __future__ import annotations -import re +import importlib.util +import sys from dataclasses import replace from pathlib import Path +from types import ModuleType import pytest +def _load_windows_stable_path_checker(root: Path) -> ModuleType: + """Import scripts/check_windows_stable_path_owner.py as a module. + + This is the single scan owner for the Windows stable executable + path boundary (owner presence + duplicate-derivation detection). + Both this test and scripts/lint-architecture-boundaries.sh (AC8) + consume it directly instead of re-implementing its regexes, globs, + or exemption handling. + """ + module_name = "check_windows_stable_path_owner" + script_path = root / "scripts" / f"{module_name}.py" + spec = importlib.util.spec_from_file_location(module_name, script_path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + + def test_plural_targets_drive_bundle_filtering(tmp_path: Path) -> None: """The canonical manifest target list must control bundle packing.""" from apm_cli.bundle.packer import pack_bundle @@ -203,39 +224,25 @@ def test_dependency_winner_selection_has_one_algorithm() -> None: def test_windows_stable_executable_path_has_one_canonical_owner() -> None: - """install.ps1 alone may define the stable current/apm.exe location.""" + """install.ps1 alone may define the stable current/apm.exe location. + + The Windows stable-path boundary (owner presence + duplicate + derivation) is scanned by exactly one checker, + scripts/check_windows_stable_path_owner.py. This test imports and + calls that checker directly -- it must not re-implement its + regexes, globs, or exemption handling -- and separately asserts + that the Bash AC8 guard actually shells out to it rather than + retaining a parallel scan. + """ root = Path(__file__).parents[2] - installer = (root / "install.ps1").read_text(encoding="utf-8") guard = (root / "scripts/lint-architecture-boundaries.sh").read_text() - assert '$currentDir = Join-Path $installRoot "current"' in installer - assert '$currentExe = Join-Path $currentDir "apm.exe"' in installer - assert "Add-ToUserPath -PathEntry $currentDir" in installer assert "Windows stable executable path belongs to install.ps1" in guard + assert "check_windows_stable_path_owner.py" in guard - join_path_current = re.compile(r'Join-Path\s+\S+\s+["\']current["\']') - literal_stable_exe = re.compile(r"current[\\/]apm\.exe") - - def derives_stable_path(text: str) -> bool: - return bool(join_path_current.search(text) or literal_stable_exe.search(text)) - - guarded_windows_scripts = [ - path - for path in (root / "scripts/windows").glob("*.ps1") - if not path.name.startswith("test-") - ] - guarded_python_files = list((root / "src/apm_cli").rglob("*.py")) - guarded_workflow_files = list((root / ".github/workflows").glob("*.yml")) + list( - (root / ".github/workflows").glob("*.yaml") - ) - - duplicate_owners = [ - path.relative_to(root).as_posix() - for path in (*guarded_windows_scripts, *guarded_python_files, *guarded_workflow_files) - if derives_stable_path(path.read_text(encoding="utf-8")) - ] + checker = _load_windows_stable_path_checker(root) - assert duplicate_owners == [] + assert checker.check(root) == [] def test_tls_injection_has_one_canonical_authority() -> None: diff --git a/tests/unit/scripts/__init__.py b/tests/unit/scripts/__init__.py new file mode 100644 index 000000000..2775971a1 --- /dev/null +++ b/tests/unit/scripts/__init__.py @@ -0,0 +1 @@ +"""Unit tests for standalone scripts/*.py checkers.""" diff --git a/tests/unit/scripts/test_check_windows_stable_path_owner.py b/tests/unit/scripts/test_check_windows_stable_path_owner.py new file mode 100644 index 000000000..5d9a43afe --- /dev/null +++ b/tests/unit/scripts/test_check_windows_stable_path_owner.py @@ -0,0 +1,230 @@ +"""TDD unit tests for the canonical Windows stable-path owner checker. + +These tests build small, hermetic fake repository trees under +`tmp_path` rather than touching the real repository, so they exercise +the checker's logic (owner presence, duplicate detection shapes, +exemption, exclusion, nested discovery, CLI behavior) in isolation. +The real tree is separately asserted clean by +`tests/integration/test_architecture_authorities.py`. +""" + +from __future__ import annotations + +import importlib.util +import subprocess +import sys +from pathlib import Path +from types import ModuleType + +import pytest + +SCRIPT_PATH = Path(__file__).resolve().parents[3] / "scripts" / "check_windows_stable_path_owner.py" +_MODULE_NAME = "check_windows_stable_path_owner" + +VALID_INSTALL_PS1 = """\ +# install.ps1 (fake, minimal) +function Add-ToUserPath { + param([string]$PathEntry) +} + +$installRoot = "C:\\Users\\example\\.apm" +$currentDir = Join-Path $installRoot "current" +$currentExe = Join-Path $currentDir "apm.exe" +Add-ToUserPath -PathEntry $currentDir +""" + + +def _load_checker() -> ModuleType: + """Import the checker script as a module without sys.path tricks.""" + spec = importlib.util.spec_from_file_location(_MODULE_NAME, SCRIPT_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + # Register before exec so dataclasses (which look up the defining + # module in sys.modules) can resolve it during class creation. + sys.modules[_MODULE_NAME] = module + spec.loader.exec_module(module) + return module + + +checker = _load_checker() + + +def _make_valid_repo(root: Path) -> None: + """Populate `root` with a minimal, checker-clean fake repository.""" + (root / "install.ps1").write_text(VALID_INSTALL_PS1, encoding="utf-8") + (root / "src" / "apm_cli").mkdir(parents=True, exist_ok=True) + (root / ".github" / "workflows").mkdir(parents=True, exist_ok=True) + (root / "scripts" / "windows").mkdir(parents=True, exist_ok=True) + + +def test_owner_statements_present_reports_no_violations(tmp_path: Path) -> None: + """A well-formed install.ps1 must not raise owner violations.""" + _make_valid_repo(tmp_path) + + assert checker.find_owner_violations(tmp_path) == [] + + +def test_owner_statement_missing_is_reported(tmp_path: Path) -> None: + """Dropping a required owner statement must produce one diagnostic.""" + _make_valid_repo(tmp_path) + (tmp_path / "install.ps1").write_text( + VALID_INSTALL_PS1.replace("Add-ToUserPath -PathEntry $currentDir\n", ""), + encoding="utf-8", + ) + + violations = checker.find_owner_violations(tmp_path) + + assert len(violations) == 1 + assert "Add-ToUserPath -PathEntry $currentDir" in violations[0] + + +def test_missing_install_ps1_is_reported(tmp_path: Path) -> None: + """A repository with no install.ps1 at all must fail loudly.""" + (tmp_path / "src" / "apm_cli").mkdir(parents=True, exist_ok=True) + + violations = checker.find_owner_violations(tmp_path) + + assert len(violations) == 1 + assert "install.ps1" in violations[0] + + +def test_positional_join_path_duplicate_is_detected(tmp_path: Path) -> None: + """Join-Path $x "current" (positional form) is a duplicate derivation.""" + _make_valid_repo(tmp_path) + culprit = tmp_path / "src" / "apm_cli" / "rogue.py" + culprit.write_text('bad = Join-Path $installRoot "current"\n', encoding="utf-8") + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == culprit + assert hits[0].line_no == 1 + + +def test_named_parameter_join_path_duplicate_is_detected(tmp_path: Path) -> None: + """Join-Path -Path $x -ChildPath "current" (named form) is a duplicate.""" + _make_valid_repo(tmp_path) + culprit = tmp_path / "scripts" / "windows" / "rogue.ps1" + culprit.write_text( + '$dup = Join-Path -Path $installRoot -ChildPath "current"\n', + encoding="utf-8", + ) + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == culprit + + +def test_named_parameter_join_path_duplicate_reordered_is_detected(tmp_path: Path) -> None: + """The -ChildPath / -Path named arguments may appear in either order.""" + _make_valid_repo(tmp_path) + culprit = tmp_path / "scripts" / "windows" / "rogue.ps1" + culprit.write_text( + '$dup = Join-Path -ChildPath "current" -Path $installRoot\n', + encoding="utf-8", + ) + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == culprit + + +@pytest.mark.parametrize("separator", ["\\", "/"]) +def test_literal_stable_path_duplicate_is_detected(tmp_path: Path, separator: str) -> None: + """A literal current\\apm.exe or current/apm.exe path is a duplicate.""" + _make_valid_repo(tmp_path) + culprit = tmp_path / "src" / "apm_cli" / "rogue.py" + culprit.write_text(f'exe = "current{separator}apm.exe"\n', encoding="utf-8") + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == culprit + + +def test_exemption_marker_suppresses_a_duplicate_line(tmp_path: Path) -> None: + """A line-level architecture-authority-exempt marker is honored.""" + _make_valid_repo(tmp_path) + exempted = tmp_path / "src" / "apm_cli" / "rogue.py" + exempted.write_text( + 'exe = "current/apm.exe" # architecture-authority-exempt: demo\n', + encoding="utf-8", + ) + + assert checker.find_duplicate_hits(tmp_path) == [] + + +def test_test_prefixed_windows_script_is_excluded(tmp_path: Path) -> None: + """Black-box validators named test-*.ps1 are not scanned as owners.""" + _make_valid_repo(tmp_path) + validator = tmp_path / "scripts" / "windows" / "test-install-script.ps1" + validator.write_text('$dup = Join-Path $prefix.Root "current"\n', encoding="utf-8") + + assert checker.find_duplicate_hits(tmp_path) == [] + + +def test_nested_workflow_file_is_discovered(tmp_path: Path) -> None: + """Workflow files nested under subdirectories must still be scanned.""" + _make_valid_repo(tmp_path) + nested = tmp_path / ".github" / "workflows" / "shared" / "nested.yml" + nested.parent.mkdir(parents=True, exist_ok=True) + nested.write_text('run: echo "current/apm.exe"\n', encoding="utf-8") + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == nested + + +def test_yaml_extension_workflow_file_is_discovered(tmp_path: Path) -> None: + """Both .yml and .yaml workflow extensions must be scanned.""" + _make_valid_repo(tmp_path) + workflow = tmp_path / ".github" / "workflows" / "rogue.yaml" + workflow.write_text('run: echo "current/apm.exe"\n', encoding="utf-8") + + hits = checker.find_duplicate_hits(tmp_path) + + assert len(hits) == 1 + assert hits[0].path == workflow + + +def test_check_is_clean_on_a_well_formed_tree(tmp_path: Path) -> None: + """The combined check() must report nothing for a clean fake tree.""" + _make_valid_repo(tmp_path) + + assert checker.check(tmp_path) == [] + + +def test_cli_exits_nonzero_and_prints_diagnostic_on_violation(tmp_path: Path) -> None: + """The CLI entry point must fail loudly with an actionable message.""" + _make_valid_repo(tmp_path) + culprit = tmp_path / "src" / "apm_cli" / "rogue.py" + culprit.write_text('exe = "current/apm.exe"\n', encoding="utf-8") + + result = subprocess.run( + [sys.executable, str(SCRIPT_PATH), "--root", str(tmp_path)], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "current/apm.exe" in result.stdout + assert "rogue.py" in result.stdout + + +def test_cli_exits_zero_and_prints_success_on_clean_tree(tmp_path: Path) -> None: + """The CLI entry point must succeed silently-ish on a clean tree.""" + _make_valid_repo(tmp_path) + + result = subprocess.run( + [sys.executable, str(SCRIPT_PATH), "--root", str(tmp_path)], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0 + assert "clean" in result.stdout From 24269354218c524daadfbef2a744da0cfb8a4cf0 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 23:17:32 +0200 Subject: [PATCH 12/12] test(windows): persist stable path owner contract Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7955c89b-a997-42aa-9c45-ef4c7fe4b1e7 --- .../instructions/architecture.instructions.md | 1 + apm.lock.yaml | 4 +- scripts/check_windows_stable_path_owner.py | 5 ++- .../test_architecture_authorities.py | 41 +++++++++++++++++++ .../test_check_windows_stable_path_owner.py | 20 +++++++++ 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/.apm/instructions/architecture.instructions.md b/.apm/instructions/architecture.instructions.md index c30293ffc..c24212159 100644 --- a/.apm/instructions/architecture.instructions.md +++ b/.apm/instructions/architecture.instructions.md @@ -39,6 +39,7 @@ instead of re-deriving the answer locally. | Install success / failure outcome | the canonical install-outcome path | | Neutral hook shape -> per-target native | the neutral hook IR + per-target integrators | | File-level deploy / sync / cleanup | BaseIntegrator (see integrators.instructions.md) | +| Windows stable executable path | install.ps1 ($currentDir / $currentExe) | If you are about to compute one of these locally, stop and call the owner. If the owner is missing a case you need, EXTEND the owner -- diff --git a/apm.lock.yaml b/apm.lock.yaml index ea3539e5c..f4576db8f 100644 --- a/apm.lock.yaml +++ b/apm.lock.yaml @@ -2772,7 +2772,7 @@ deployments: owners: - . active_owner: . - content_hash: sha256:a24c5c8ce4f50855a10e4b6c0814ab8e6faa4bdb0aaa715f6cf7a5fe6f253121 + content_hash: sha256:af293e32eb828c69989d873d9f0b87f8c8204819654bbb216afa9a70c03f13fe - kind: project-relative target: copilot value: .github/instructions/changelog.instructions.md @@ -3279,7 +3279,7 @@ local_deployed_file_hashes: .github/agents/spec-tag-architect.agent.md: sha256:82907265c5e7cf1ac61ad96866fa7c5683b69c8f09b7a4c5f3cc241acc9568ca .github/agents/supply-chain-security-expert.agent.md: sha256:8fb8cc426d6af17ba084a28b3f026c2b475b62e3ca63ed2f88b83bd823f877af .github/agents/test-coverage-expert.agent.md: sha256:bc588d89530362469502bfbea788df892a9a0b00e630cd0f3926d3dfd2c2a9e2 - .github/instructions/architecture.instructions.md: sha256:a24c5c8ce4f50855a10e4b6c0814ab8e6faa4bdb0aaa715f6cf7a5fe6f253121 + .github/instructions/architecture.instructions.md: sha256:af293e32eb828c69989d873d9f0b87f8c8204819654bbb216afa9a70c03f13fe .github/instructions/changelog.instructions.md: sha256:1e51ec4c74e847967962bd279dc4c6e582c5d3578490b3c28d5f3acd3e05f73e .github/instructions/cicd.instructions.md: sha256:08d87b7d635761cb41deb8fc71d5d83f54678de463db484afb16d2d4f8713ecb .github/instructions/cli.instructions.md: sha256:8e39e8d5047ce88575cb02f87c2bcede584dfef258bd86f7466c7badf136541a diff --git a/scripts/check_windows_stable_path_owner.py b/scripts/check_windows_stable_path_owner.py index 5eec87358..07395094b 100644 --- a/scripts/check_windows_stable_path_owner.py +++ b/scripts/check_windows_stable_path_owner.py @@ -71,7 +71,10 @@ EXEMPT_MARKER = "architecture-authority-exempt:" # Literal stable-path forms: current\apm.exe (Windows) or current/apm.exe. -_LITERAL_STABLE_EXE = re.compile(r"current[\\/]apm\.exe") +# The leading \b prevents matching a "current" that is a suffix of a longer +# identifier (e.g. concurrent/apm.exe, mycurrent/apm.exe) while still +# matching both path separators for a real standalone "current" segment. +_LITERAL_STABLE_EXE = re.compile(r"\bcurrent[\\/]apm\.exe") # A Join-Path call is a duplicate derivation if it appears on the same # line as a quoted "current" (single or double quotes), regardless of diff --git a/tests/integration/test_architecture_authorities.py b/tests/integration/test_architecture_authorities.py index 90503e8d4..166cd869a 100644 --- a/tests/integration/test_architecture_authorities.py +++ b/tests/integration/test_architecture_authorities.py @@ -245,6 +245,47 @@ def test_windows_stable_executable_path_has_one_canonical_owner() -> None: assert checker.check(root) == [] +def test_windows_owner_row_stays_synced_source_deployed_and_lockfile() -> None: + """The new owner-table row must not silently drop on the next deploy. + + ``.github/instructions/architecture.instructions.md`` is a compiled + artifact: ``.apm/instructions/architecture.instructions.md`` is its + canonical compile source (see docs/src/content/docs/producer/compile.md), + and apm.lock.yaml records a content hash of the deployed copy. If the + deployed file gains a row that the source lacks, the next + ``apm compile`` / ``apm install`` would regenerate the deployed file + from the (stale) source and silently remove the row; a stale lockfile + hash would additionally make ``apm audit`` report drift. This guards + all three legs of that contract using the project's own lockfile codec + and content-hash function rather than a bespoke comparison. + """ + root = Path(__file__).parents[2] + source = root / ".apm/instructions/architecture.instructions.md" + deployed = root / ".github/instructions/architecture.instructions.md" + + owner_row = "| Windows stable executable path | install.ps1 ($currentDir / $currentExe) |" + assert owner_row in source.read_text(encoding="utf-8") + + # Source and deployed must be byte-identical: the deployed file is a + # compiled copy of the source, not an independently edited artifact. + assert source.read_bytes() == deployed.read_bytes() + + from apm_cli.core.deployment_ledger import DeploymentLedgerCodec + from apm_cli.deps.lockfile import LockFile + from apm_cli.utils.content_hash import compute_file_hash + + lockfile = LockFile.load_or_create(root / "apm.lock.yaml") + ledger = DeploymentLedgerCodec.from_lockfile(lockfile) + locator_key = "copilot||project|.github/instructions/architecture.instructions.md" + record = ledger.records.get(locator_key) + + assert record is not None, "lockfile must track the deployed architecture instruction" + assert record.content_hash == compute_file_hash(deployed), ( + "apm.lock.yaml content_hash is stale relative to the deployed file; " + "the next 'apm audit' would report hash drift" + ) + + def test_tls_injection_has_one_canonical_authority() -> None: """Only the parent TLS owner and standalone child bootstrap may inject.""" root = Path(__file__).parents[2] diff --git a/tests/unit/scripts/test_check_windows_stable_path_owner.py b/tests/unit/scripts/test_check_windows_stable_path_owner.py index 5d9a43afe..d920eaf14 100644 --- a/tests/unit/scripts/test_check_windows_stable_path_owner.py +++ b/tests/unit/scripts/test_check_windows_stable_path_owner.py @@ -144,6 +144,26 @@ def test_literal_stable_path_duplicate_is_detected(tmp_path: Path, separator: st assert hits[0].path == culprit +@pytest.mark.parametrize("separator", ["\\", "/"]) +def test_similarly_named_identifier_is_not_a_false_positive(tmp_path: Path, separator: str) -> None: + """ "concurrent/apm.exe" must not be mistaken for the stable "current" path. + + The literal-path branch matched "current[\\/]apm.exe" anywhere in the + line, so a substring like "concurrent/apm.exe" (or the Windows- + separator form) tripped a false positive purely because "concurrent" + contains "current" as a substring. The check must require "current" + to start at a word boundary so unrelated identifiers ending in + "current" are left alone, while both real path separators for an + actual standalone "current" segment keep matching (see the + parametrized ``test_literal_stable_path_duplicate_is_detected`` above). + """ + _make_valid_repo(tmp_path) + innocent = tmp_path / "src" / "apm_cli" / "innocent.py" + innocent.write_text(f'path = "concurrent{separator}apm.exe"\n', encoding="utf-8") + + assert checker.find_duplicate_hits(tmp_path) == [] + + def test_exemption_marker_suppresses_a_duplicate_line(tmp_path: Path) -> None: """A line-level architecture-authority-exempt marker is honored.""" _make_valid_repo(tmp_path)