Skip to content

fix(installer): expose stable Windows executable#2094

Open
danielmeppiel wants to merge 9 commits into
mainfrom
fix/windows-apm-exe-shim
Open

fix(installer): expose stable Windows executable#2094
danielmeppiel wants to merge 9 commits into
mainfrom
fix/windows-apm-exe-shim

Conversation

@danielmeppiel

Copy link
Copy Markdown
Collaborator

fix(installer): expose stable Windows executable

TL;DR

The Windows installer now publishes the complete active PyInstaller bundle through a version-stable current junction and adds that directory to PATH. This gives Git Bash and Python subprocess.run(["apm", ...]) a real apm.exe to resolve while preserving the existing apm.cmd launcher for command shells.

Note

Closes #2076.

Problem (WHY)

  • The installer previously put only bin\apm.cmd on PATH; Git Bash does not resolve that extension as apm.
  • Windows CreateProcess callers such as Python's subprocess.run(["apm", ...]) do not use cmd.exe PATHEXT expansion, so they failed with WinError 2.
  • [!] Pointing callers at releases\<version>\apm.exe exposed the installer's versioned internal layout and became stale after updates.

Issue #2076 provides the observed Git Bash and Python reproductions. The fix keeps the documented install command stable across Windows caller types.

Approach (WHAT)

# Fix
1 Create an installRoot\current directory junction targeting the promoted release bundle.
2 Replace the junction with rollback protection and refuse to overwrite a non-junction path.
3 Add both bin and current to user and process PATH; keep apm.cmd for compatibility.
4 Gate releases with bare Python subprocess and Git Bash resolution checks.
5 Document the Windows layout in installation, self-update, and packaged usage guidance.

Implementation (HOW)

File Change
install.ps1 Creates and safely swaps the current junction after release promotion, verifies current\apm.exe, and adds the stable executable directory to PATH.
scripts/windows/test-install-script.ps1 Extends the Windows installer E2E gate with the exact Python and Git Bash regressions from #2076.
tests/unit/test_windows_installer_launchers.py Adds a fast regression trap ensuring the installer and release gate retain stable executable resolution.
docs/src/content/docs/getting-started/installation.md Documents the Windows bin plus current layout and subprocess behavior.
docs/src/content/docs/reference/cli/self-update.md Explains how self-update advances the stable executable junction.
packages/apm-guide/.apm/skills/apm-usage/installation.md Keeps packaged agent installation guidance synchronized.
CHANGELOG.md Records the Windows resolution fix under Unreleased.

Diagram

Legend: the dashed nodes are the new stable executable path; the existing command-shell path remains intact.

flowchart LR
    subgraph Install[Windows installer]
        R["releases/version bundle"]
        C["current junction"]
        B["bin/apm.cmd"]
    end
    subgraph Callers[Callers]
        CMD["cmd.exe and PowerShell"]
        GB["Git Bash"]
        PY["Python subprocess"]
    end
    R --> C
    R --> B
    B --> CMD
    C --> GB
    C --> PY
    classDef new stroke-dasharray: 5 5;
    class C,GB,PY new;
Loading

Trade-offs

  • Junction over copied bundle. A junction keeps apm.exe beside its PyInstaller runtime files and avoids duplicating every release into bin.
  • Junction over a generated executable shim. The installer needs no compiler or additional unsigned launcher binary, reducing endpoint-policy and antivirus risk.
  • Two PATH entries over replacing bin. Existing apm.cmd behavior remains compatible while non-command-shell callers gain apm.exe.
  • Fail closed on path collision. A real file or directory at current is not deleted; the installer reports the conflict instead of risking user data.

Benefits

  1. subprocess.run(["apm", "--version"]) resolves a real executable on Windows.
  2. command -v apm and apm --version work in Git Bash after installation.
  3. Callers use one stable path across upgrades instead of discovering release-version directories.
  4. Existing apm.cmd invocations continue unchanged.

Validation

uv run --extra dev pytest tests/unit/install/test_windows_shim_template.py tests/unit/test_docker_args_and_installer.py tests/unit/test_enterprise_bootstrap_installers.py tests/unit/test_safe_installer.py tests/unit/test_windows_installer_launchers.py -q:

...............................................                          [100%]
47 passed in 5.56s

Full CI lint mirror (ruff, formatting, YAML I/O, file length, portable relative paths, pylint duplication, auth boundary):

All checks passed!
1411 files already formatted
Your code has been rated at 10.00/10
[+] auth-signal lint clean
CI grep guard equivalents passed

PowerShell parser validation for install.ps1 and scripts/windows/test-install-script.ps1 exited 0.

TDD and mutation-break evidence

The regression test first failed because install.ps1 did not define the stable current executable path. Removing the junction creation from the completed implementation then produced:

FAILED tests/unit/test_windows_installer_launchers.py::test_windows_installer_exposes_stable_executable_on_path
assert 'New-Item -ItemType Junction' in installer
1 failed, 1 passed

Restoring the junction creation returned the installer suite to 47 passing tests.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 Python can launch bare apm after a Windows install DevX (pragmatic as npm) scripts/windows/test-install-script.ps1::Test-EndToEndInstall (regression-trap for #2076) e2e
2 Git Bash resolves and runs bare apm after a Windows install DevX (pragmatic as npm), Multi-harness support scripts/windows/test-install-script.ps1::Test-EndToEndInstall (regression-trap for #2076) e2e
3 The release gate cannot silently drop the stable executable launcher DevX (pragmatic as npm) tests/unit/test_windows_installer_launchers.py::test_windows_installer_exposes_stable_executable_on_path unit

How to test

  • On Windows, run install.ps1 and confirm both bin\apm.cmd and current\apm.exe exist.
  • Start Git Bash and run command -v apm && apm --version; expect the active version.
  • Run python -c "import subprocess; subprocess.run(['apm', '--version'], check=True)"; expect exit code 0.
  • Install a newer version and repeat both calls; expect the new version without changing either command.
  • Place a real directory at current and rerun the installer; expect a safe failure without deleting it.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Windows executable resolution for non-cmd.exe callers by introducing a version-stable current junction that exposes the active PyInstaller onedir bundle (including apm.exe) and adding that directory to PATH, while keeping the existing bin\\apm.cmd shim for shell compatibility.

Changes:

  • Update install.ps1 to create/swap a current junction pointing at the promoted release bundle and add current to PATH.
  • Extend the Windows E2E installer gate to validate Git Bash resolution and a bare Python subprocess.run(["apm", ...]) flow.
  • Add unit-level regression traps and update docs/guide/changelog to describe the Windows bin + current layout.
Show a summary per file
File Description
install.ps1 Creates a stable current junction to the active release bundle and adds it to PATH; verifies current\\apm.exe exists.
scripts/windows/test-install-script.ps1 Adds E2E coverage for Python CreateProcess-style resolution and Git Bash command resolution.
tests/unit/test_windows_installer_launchers.py Adds unit tests that assert the installer and E2E gate include the stable-exe/junction behaviors.
docs/src/content/docs/getting-started/installation.md Documents that Windows exposes bin\\apm.cmd plus stable current\\apm.exe for Git Bash/subprocess callers.
docs/src/content/docs/reference/cli/self-update.md Updates self-update docs to mention advancing the Windows current junction and PATH behavior.
packages/apm-guide/.apm/skills/apm-usage/installation.md Syncs packaged installation guidance with the new current directory behavior.
CHANGELOG.md Adds an Unreleased entry describing the Windows resolution fix.

Review details

  • Files reviewed: 7/7 changed files
  • Comments generated: 3
  • Review effort level: Low

Comment on lines +341 to +344
$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"

Comment on lines +348 to +351
$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"
}
Comment thread CHANGELOG.md Outdated
Comment on lines +12 to +14
- 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)
danielmeppiel and others added 3 commits July 10, 2026 04:06
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>
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>
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>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Windows installation now provides stable bare apm resolution across cmd.exe, Python subprocess, and Git Bash without weakening upgrade safety.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Every active specialist cleared the final diff. The architectural and security signals converge on the same result: the checksum-verified onedir bundle remains intact, current\apm.exe gives non-PATHEXT callers a stable executable, non-junction collisions fail safely, and unattended upgrades remove only the stale junction object.

The evidence is load-bearing. Exact-SHA Windows execution proved a path containing spaces and &, bare Python subprocess resolution, Git Bash resolution, cross-version upgrade, same-version reinstall, and self-update. Mutation breaks proved both junction creation and safe old-junction deletion are required by the regression trap.

Aligned with: Secure by default through collision refusal and verified promotion; Multi-harness support through cmd.exe, Python, and Git Bash coverage; OSS community-driven through a regression test for #2076; pragmatic as npm through bare PATH invocation.

Growth signal. This removes a concrete Windows first-run barrier for shell and automation users.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 0 No remaining architectural issues; the guarded installer pipeline and stable executable contract are fully exercised.
CLI Logging Expert 0 0 0 Collision errors are actionable and success output identifies both launch paths.
DevX UX Expert 0 0 0 Stable apm.exe works without disturbing the existing apm.cmd flow.
Supply Chain Security Expert 0 0 0 Verified promotion and regular-directory collision refusal preserve the trust boundary.
OSS Growth Hacker 0 0 0 Windows interoperability is clearly documented and release-ready.
Doc Writer 0 0 0 Changed installation, self-update, packaged guide, and changelog claims match behavior.
Test Coverage Expert 0 0 0 Exact Windows E2E and two mutation breaks close the regression surface.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Architecture

flowchart TD
    A["Verified release bundle"] --> B["Promote versioned release"]
    B --> C["Create current.new junction"]
    C --> D{"Existing current path?"}
    D -->|junction| E["Move old junction aside"]
    D -->|regular path| F["Fail safely"]
    D -->|absent| G["Promote current.new"]
    E --> G
    G --> H["Delete old junction object"]
    H --> I["Add bin and current to PATH"]
    I --> J["cmd.exe uses apm.cmd"]
    I --> K["Git Bash and subprocess use apm.exe"]
Loading

Folded in this run

  • (Copilot) Escape pinned versions before PowerShell regex matching -- resolved in ade13054afde0bba113aa66a81a28513ad55c905.
  • (Copilot) Include PR traceability in the changelog entry -- resolved in ade13054afde0bba113aa66a81a28513ad55c905.
  • (Panel) Exercise launcher quoting from a path containing spaces and a cmd metacharacter -- resolved in ade13054afde0bba113aa66a81a28513ad55c905.
  • (Panel) Make non-junction collision guidance actionable and clarify PATH intent -- resolved in ade13054afde0bba113aa66a81a28513ad55c905.
  • (Panel) Surface focused Windows installer evidence before unrelated broad unit failures -- resolved in 71217417e68f7128699c266af5eb0bd1c939c86c.
  • (Windows E2E) Remove stale junctions without a Windows PowerShell 5.1 NonInteractive prompt -- resolved in 8f7d6d4dc68cf1fb9ac10378d52dedee1649f293.

Copilot signals reviewed

  • scripts/windows/test-install-script.ps1:353 -- LEGIT: pinned Python output was treated as a regex; escaped and verified.
  • scripts/windows/test-install-script.ps1:362 -- LEGIT: pinned Git Bash output was treated as a regex; escaped and verified.
  • CHANGELOG.md -- LEGIT: changelog needed PR traceability; issue and PR references are now present.

Regression-trap evidence (mutation-break gate)

  • tests/unit/test_windows_installer_launchers.py::test_windows_installer_exposes_stable_executable_on_path -- deleted New-Item -ItemType Junction; test FAILED as expected; guard restored.
  • tests/unit/test_windows_installer_launchers.py::test_windows_installer_exposes_stable_executable_on_path -- deleted [System.IO.Directory]::Delete($oldCurrentDir); test FAILED as expected; guard restored.

Validation

  • Local installer suite: 47 passed.
  • Full lint mirror: ruff check and format silent; pylint R0801, YAML I/O, file length, portable path, and auth-signal guards clean.
  • Exact-SHA PR CI: every reported check succeeded or skipped as designed.
  • Exact-SHA Windows job: focused installer gate ended with All install.ps1 integration tests passed. The later job failure is in pre-existing MCP tests outside this diff.

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2094 8f7d6d4 ship_now 3 6 0 2 green MERGEABLE BLOCKED required review pending

Recommendation

Ship now. The exact final commit is green in required CI, the focused Windows installer gate passed on a real Windows runner, all in-scope findings are folded, and no panel follow-ups remain.


Full per-persona findings

All active panelists returned no findings on the final SHA. Auth and performance lenses were inactive because the diff touches neither authentication nor package-manager performance paths.

This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Windows installation now has a committed integration-tier regression gate for stable bare apm resolution across Python subprocess and Git Bash.

cc @sergio-sisternes-epam - this fresh advisory reviews exact SHA 6cbfa84e3051173cca56fa45b6464d1643290f53.

All substantive specialist signals converge. The new tests/integration/test_windows_installer_launchers.py invokes the real PowerShell installer E2E on a Windows runner, and the release workflow runs that exact pytest test. Architecture, security, DevX, documentation, and coverage lenses found no remaining substantive follow-ups. Two optional comment/logging nits do not affect behavior.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 0 Integration wrapper and declarative Windows gating are clean.
CLI Logging Expert 0 0 1 Existing installer output is actionable; target diagnostics are optional.
DevX UX Expert 0 0 1 Stable executable behavior and collision recovery are clear.
Supply Chain Security Expert 0 0 0 Junction swap and collision refusal preserve the trust boundary.
OSS Growth Hacker 0 0 0 Windows launcher reliability removes an adoption barrier.
Doc Writer 0 0 0 User and contributor docs match behavior.
Test Coverage Expert 0 0 0 Real Windows E2E plus mutation proof closes the integration-tier gap.

B = blocking-severity findings, R = recommended, N = nits. Counts are advisory signal strength.

Architecture

flowchart TD
    A["Windows build workflow"] --> B["pytest integration test"]
    B --> C["PowerShell installer E2E"]
    C --> D["current/apm.exe"]
    D --> E["Python subprocess bare apm"]
    D --> F["Git Bash bare apm"]
Loading

Regression-trap evidence

  • Exact final SHA Windows execution: Test install.ps1 end-to-end (Windows) succeeded.
  • Mutation: temporary SHA da12924ea230a5c4832123386e383b68ef94b1f7 deleted production guard New-Item -ItemType Junction.
  • Result: the exact integration test failed on Windows (1 failed in 93.11s): mutation run.
  • Guard restored at final SHA.

Validation

  • Full local CI lint mirror passed: ruff check, format, YAML I/O, file length, portable paths, pylint duplication, and auth boundary.
  • Targeted local tests: 2 unit tests passed; integration test collected and declaratively skipped on non-Windows.
  • Exact-final-SHA PR checks are green, including Lint, both test shards, coverage, CodeQL, binary smoke, spec conformance, and the merge gate.

Recommendation

ship_now. Exact-final-SHA CI is green, the real Windows integration boundary passed, the production-guard mutation broke the exact integration test, and no in-scope follow-up remains.

This panel is advisory. The maintainer and author decide when to ship.

danielmeppiel and others added 4 commits July 10, 2026 21:12
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
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
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Windows executable resolution fix is complete, well-defended, and ready to ship -- every in-scope panel finding is folded, and the two accepted trade-offs carry clear rationale.

cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Seven active panelists converged on a clean bill. The critical coverage gap -- upgrade/reinstall tests (4a/4b) never verifying that the junction actually points at the NEW release -- was caught independently by both python-architect and test-coverage-expert and has been folded with a new-version assertion that traps wrong-target junctions. Supply-chain flagged a theoretical TOCTOU between the reparse-point check and Move-Item; this is a single-user bootstrap path where an attacker with write access to the install root already owns the machine, so the risk is accepted without a follow-up. The onedir-DLL PATH concern is similarly accepted: the bundle directory carries the same privilege boundary as binDir, and no additional attack surface is introduced beyond what the existing shim already grants.

The only substantive disagreement was devx-ux wanting to trim the installer's success output (two paths printed) and flag "junction" as jargon. Both calls were resolved: the dual-path output directly reassures the Git Bash and Python subprocess users this PR unblocks (growth concurs), and "junction" is the precise NTFS term needed to explain self-update semantics (doc-writer domain authority).

Strategically, this PR closes the last platform-parity gap in APM's launcher story. A bare apm invocation now resolves identically across cmd.exe, PowerShell, Git Bash, and Python subprocess on Windows -- the exact surface where CreateProcess's PATHEXT-blind resolution was failing users. The junction-swap state machine is sound, the rollback path is guarded, and the mutation-break proof confirms the guard is load-bearing.

Dissent. devx-ux recommended trimming the installer success line that prints both the shim path and the stable executable path; oss-growth-hacker argued it is a positive first-run confidence signal for the exact users affected by #2076. Sided with growth: the line costs nothing and reassures the right audience. devx-ux also flagged "junction" as jargon in docs; doc-writer holds domain authority and the term is technically precise.

Aligned with: portable-by-manifest (the junction gives Windows full parity with Unix symlink resolution -- one apm command on every platform APM targets); pragmatic-as-npm (after install apm just works -- no PATHEXT config, no .cmd suffix, no shell-specific workarounds); multi-harness-multi-host (validated across CreateProcess, Git Bash, and cmd.exe, over paths with spaces and shell metacharacters).

Growth signal. "Bare apm.exe now resolves in Git Bash and Python subprocess on Windows" is a concrete DevX win that speaks to the CI/automation audience APM targets. Defer the release-note highlight to the cut-release skill, but mark it a must-include story angle.

Panel summary

Persona B R N Takeaway
Python Architect 0 1 2 Junction state machine sound; upgrade/reinstall tests needed installRoot-scoped junction-temp assertions (folded).
Test Coverage Expert 0 1 0 Fresh-install resolution well-defended; upgrade tests now trap wrong-target junctions after swap (folded).
Supply Chain Security Expert 0 0 3 No blocking risk; onedir-DLL and TOCTOU noted as accepted trade-offs.
CLI Logging Expert 0 0 1 Non-junction error no longer prints the path twice (folded).
DevX UX Expert 0 2 2 PATH-order comment corrected; success-line and docs jargon kept by design.
Doc Writer 0 0 2 All doc claims accurate; passive voice and duplicate heading folded.
OSS Growth Hacker 0 1 1 Concrete Windows DevX win worth a release-note highlight.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 4 follow-ups

  1. [OSS Growth Hacker] Release-note highlight for Windows executable resolution -- deferred as out-of-diff release narrative, but "bare apm resolving in Git Bash and subprocess" is a concrete DevX win worth amplifying at the next cut-release.
  2. [Supply Chain Security Expert] Accepted trade-off: onedir bundle DLLs on PATH -- same privilege boundary as binDir; the alternative (copying apm.exe out of the bundle) breaks PyInstaller's co-located DLL contract.
  3. [Supply Chain Security Expert] Accepted trade-off: minor TOCTOU in junction swap -- single-user bootstrap path; attacker needs pre-existing write access to installRoot, and the refuse-non-junction guard limits blast radius.
  4. [OSS Growth Hacker] Astro callout for the Windows installation sentence -- optional polish deferred because it conflicts with the doc-writer active-voice edit on the same line.

Architecture

classDiagram
    direction LR
    class InstallPs1 {
        <<Orchestrator>>
        +main()
        -$installRoot str
        -$binDir str
        -$releasesDir str
    }
    class ReleasePromoter {
        <<TwoPhaseReplace>>
        +stage($stagingDir, $releaseDir)
        +moveAside($backupDir)
        +promote()
        +rollback()
        +cleanup()
    }
    class JunctionManager {
        <<TwoPhaseReplace>>
        +createTemp($newCurrentDir, $releaseDir)
        +guardNonJunction($currentDir)
        +swapIn($currentDir)
        +rollback()
        +cleanup()
    }
    class BinShim {
        <<Adapter>>
        +writeShim($shimPath, $releaseDir)
        +resolveLocalAppData()
    }
    class PathManager {
        <<SideEffect>>
        +AddToUserPath($binDir)
        +AddToUserPath($currentDir)
    }
    class CallerResolution {
        <<ValueObject>>
        +cmdExe_via_PATHEXT str
        +createProcess_via_PATH str
        +gitBash_via_PATH str
    }
    InstallPs1 *-- ReleasePromoter : promotes release
    InstallPs1 *-- JunctionManager : creates current junction
    InstallPs1 *-- BinShim : writes apm.cmd
    InstallPs1 *-- PathManager : configures PATH
    JunctionManager ..> ReleasePromoter : targets releaseDir
    BinShim ..> ReleasePromoter : references releaseDir
    PathManager ..> CallerResolution : enables
    note for ReleasePromoter "Two-phase replace:\nstage -> move aside -> promote -> cleanup"
    note for JunctionManager "Same two-phase pattern:\ntemp junction -> move old -> promote -> cleanup"
Loading
flowchart TD
    A["User runs: irm aka.ms/apm-windows | iex"] --> B["[NET] Download + verify asset zip"]
    B --> C["[FS] Extract to stagingDir.new-GUID"]
    C --> D["Smoke test apm.exe --version"]
    D --> E{"releaseDir exists?"}
    E -- yes --> F["[FS] Move-Item releaseDir -> releaseDir.old-GUID"]
    F --> G["[FS] Move-Item stagingDir -> releaseDir"]
    E -- no --> G
    G --> H["[FS] Remove-Item releaseDir.old-GUID"]
    H --> I["[FS] New-Item Junction current.new-GUID -> releaseDir"]
    I --> J{"current exists?"}
    J -- yes --> K{"Is ReparsePoint?"}
    K -- no --> REFUSE["THROW: refuse non-junction path"]
    K -- yes --> L["[FS] Move-Item current -> current.old-GUID"]
    L --> M["[FS] Move-Item current.new-GUID -> current"]
    J -- no --> M
    M --> N["[FS] Directory.Delete current.old-GUID (junction only)"]
    N --> O{"current/apm.exe exists?"}
    O -- no --> FAIL["exit 1: stable exe missing"]
    O -- yes --> P["[FS] Write bin/apm.cmd shim"]
    P --> Q["Add-ToUserPath bin"]
    Q --> R["Add-ToUserPath current"]
    R --> S["SUCCESS: apm installed"]
    M -- failure --> ROLLBACK["Catch: delete current.new-GUID\nrestore current.old-GUID -> current"]
    ROLLBACK --> EXIT1["exit 1"]
Loading
sequenceDiagram
    participant User
    participant CmdExe as cmd.exe / PowerShell
    participant GitBash as Git Bash
    participant Python as Python subprocess
    participant BinShim as bin/apm.cmd
    participant CurrentJunction as current/ (junction)
    participant ReleaseBundle as releases/vX.Y.Z/
    User->>CmdExe: apm --version
    CmdExe->>BinShim: PATHEXT resolves .cmd
    BinShim->>ReleaseBundle: @releases\vX.Y.Z\apm.exe %*
    ReleaseBundle-->>CmdExe: version output
    User->>GitBash: apm --version
    GitBash->>CurrentJunction: PATH lookup finds apm.exe
    CurrentJunction->>ReleaseBundle: junction target
    ReleaseBundle-->>GitBash: version output
    User->>Python: subprocess.run(["apm", "--version"])
    Python->>CurrentJunction: CreateProcessW PATH search
    CurrentJunction->>ReleaseBundle: junction target
    ReleaseBundle-->>Python: version output
Loading

Recommendation

Every in-scope finding across the active panelists has been folded and verified. The two accepted trade-offs (onedir DLLs on PATH, TOCTOU in junction swap) carry clear rationale and do not warrant blocking. The mutation-break proof confirms the junction-resolution guard is load-bearing, the upgrade/reinstall tests now trap wrong-target junctions, and all seven CI-mirror lint gates are green. CI on the Windows matrix is the sole remaining gate -- an infrastructure concern, not a code concern. Ship it.


Full per-persona findings

Python Architect

  • [recommended] Upgrade/reinstall leftover assertions scanned releasesDir but junction temps live at installRoot at scripts/windows/test-install-script.ps1
    Test-CrossVersionUpgrade and Test-SameVersionReinstall asserted no .new-*/.old-* under releasesDir, but current.new-<guid>/current.old-<guid> are created one level up at the install root. FOLDED with installRoot-scoped assertions.
  • [nit] Rollback Directory.Delete not guarded against .NET exceptions at install.ps1
    A terminating error from Delete in the catch block would skip the rollback Move-Item that restores the old junction. FOLDED: cleanup wrapped in try/catch.
  • [nit] Unit test missing from __future__ import annotations at tests/unit/test_windows_installer_launchers.py
    Convention across the codebase (and the integration counterpart). FOLDED.

Test Coverage Expert

  • [recommended] Upgrade tests never verified current\apm.exe exists or resolves the new version after junction swap at scripts/windows/test-install-script.ps1
    A wrong-target junction (still pointing at the previous release) survives the installer's own Test-Path guard. FOLDED: 4a/4b now assert the stable exe exists AND reports the new version, plus an installRoot leftover check.

Supply Chain Security Expert

  • [nit] Junction-temp leftover check gap in upgrade/reinstall tests -- FOLDED (convergent with the coverage finding above).
  • [nit] onedir bundle DLLs resolvable by bare name on PATH -- accepted trade-off: same privilege boundary as binDir; no new attack surface.
  • [nit] Minor TOCTOU between reparse-point check and Move-Item -- accepted trade-off: theoretical, single-user path, requires pre-existing write access.

CLI Logging Expert

  • [nit] Non-junction error surfaced the path twice at install.ps1
    The throw embedded the path and the catch wrapper embedded it again. FOLDED: path dropped from the throw; the catch wrapper remains the single source.

DevX UX Expert

  • [recommended] Installer success prints two paths (shim + stable exe) -- KEPT by design: deliberate first-run confidence for the [BUG] Windows: installer exposes only apm.cmd on PATH — no resolvable apm.exe, breaking non-cmd callers (Git Bash, Python subprocess) #2076-affected audience (growth concurs).
  • [recommended] "junction" jargon in user-facing docs -- DEFERRED: doc-writer (domain authority) approved the wording; the term is precise and needed to explain self-update semantics.
  • [nit] "after bin" PATH comment misleading (Add-ToUserPath prepends) at install.ps1 -- FOLDED: comment corrected to state the junction precedes bin.
  • [nit] CHANGELOG "junction" jargon -- KEPT: doc-writer approved; power-user mechanism note.

Doc Writer

  • [nit] Duplicate ### Fixed heading under [Unreleased] at CHANGELOG.md -- FOLDED: consolidated into one block.
  • [nit] Passive voice in the Windows installation sentence at docs/src/content/docs/getting-started/installation.md -- FOLDED: active-voice rewrite.

OSS Growth Hacker

  • [recommended] Elevate to a release-note highlight -- DEFERRED: release narrative, out of PR diff scope; captured as a must-include story angle for cut-release.
  • [nit] Astro callout for the Windows sentence -- DEFERRED: optional polish that conflicts with the doc-writer active-voice edit on the same line.

Auth Expert -- inactive

No authentication surface: GITHUB_TOKEN usage was only repositioned in build-release.yml, not added; no credential-resolution path touched.

Performance Expert -- inactive

No hot-path surface: install.ps1 bootstrap is not a package-manager resolver/lockfile/download path.

This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Windows: installer exposes only apm.cmd on PATH — no resolvable apm.exe, breaking non-cmd callers (Git Bash, Python subprocess)

2 participants