fix(install): shorten staging path segments to clear Windows MAX_PATH - #2941

Added in 6fc0cf1. This file marks per-test rather than at module level, so the marker went on the test rather than a module-level pytestmark.

pytest tests/unit/install/test_resolution_staging_relocate.py -m windows_compat now collects 2 of 12 rather than 1, and both pass.

The bounded staging-path repair looks aligned with the stabilization priorities, but I do not have capacity to review this one myself. I am removing my pending review request so it does not imply otherwise.

Sergio Sisternes (@sergio-sisternes-epam), can you confirm whether you have capacity to review the Windows/staging scope or coordinate a willing reviewer? The existing request for your review is not being treated as confirmed availability.

For now #2896 stays in Next. We are not adding status/accepted or issuing a formal scope-approval record without a willing review contact. Mohammed Alkindi (@MohammedAlkindi), please do not spend time on further rebases or unrelated Windows failures for our benefit while we arrange review support. The intended review remains bounded to shorter transient staging paths, collision/concurrency safety and legacy cleanup; no arbitrary long-path guarantee or broad Windows cleanup is being requested.

PR triage recommendation

needs-issue

Recommendation only, not merge or scope approval. A responsible
human maintainer must approve. Labels, automated advice, and
silence are not approval.

Linked issue

#2896 (open; priority/high, triage/recommended; not status/accepted).
A prior automated accept was withdrawn by a maintainer on 2026-09-12.
On this PR, a maintainer stated that #2896 stays in Next and that
status/accepted will not be added without a willing review contact.

Proposed classification

type/bug, theme/portability, area/distribution

Suggested next action

Wait for a responsible human maintainer to record bounded scope,
acceptance criteria, and a review contact on #2896 (or this PR)
before treating this implementation as invited.

CODEOWNERS for this tree: Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam).

Thank you for contributing this pull request. APM starts with an
issue, not an implementation
(https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md).
This PR already links #2896, but that issue is not labelled
status/accepted. Please wait for a maintainer to review and accept
the scope on #2896 first. This PR is labelled status/deferred until
that happens.

Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

Mohammed Alkindi (@MohammedAlkindi) Daniel Meppiel (@danielmeppiel) I'm accepting this and looking for support to get this reproduced and tested on Windows before merging.

APM Review Panel: needs_rework

This PR shortens transient Windows staging paths to remove a concrete install blocker while preserving collision safety and legacy cleanup.

cc Mohammed Alkindi (@MohammedAlkindi) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

The technical panel agrees the implementation is narrowly scoped, preserves the existing security and concurrency properties, and does not change authentication, CLI output, or public documentation. It aligns with the accepted #2896 scope: shorter transient staging paths, not a promise of arbitrary Windows long-path support.

However, the shared test evidence identifies a portability gap. tests/unit/install/test_resolution_staging_relocate.py::test_prepare_replacement_slot_names_fit_windows_max_path proves the replacement slot is shorter, but does not prove that a representative nested staged payload remains below the Windows path budget. Because the maintainer explicitly requested Windows reproduction and test support, add that regression assertion before treating the fix as fully evidenced. A concise Unreleased Fixed entry would also give affected users a discoverable upgrade reason.

Aligned with: Portable by manifest: shorter opaque segments improve installation from deep Windows checkouts, pending a nested-payload assertion. Secure by default: scoped entropy, locking, containment, and legacy cleanup are unchanged. Pragmatic as npm: the patch removes a platform-specific failure without widening the long-path contract.

Growth signal. A concise changelog entry gives Windows enterprise users a clear reason to upgrade when staging-path length caused installation failures.

Panel summary

B = highest-severity findings, R = recommended, N = nits. Counts communicate signal strength, not a gate.

Top 2 follow-ups

1. [Test Coverage Expert] Assert a representative nested staged file fits the Windows MAX_PATH budget -- the current test proves the opaque replacement root is shorter, not that realistic package content written beneath it remains within the path limit.
2. [OSS Growth Hacker] Add an Unreleased Fixed changelog entry for the Windows staging-path fix -- affected users need a durable upgrade reason without overstating support for arbitrary long paths.

Architecture

classDiagram
    direction LR
    class ResolutionStagingSession {
      <<RollbackJournal>>
      -_staging_root Path
      -_staging_lock FileLock
      +prepare_replacement(path) Path
      +publish_replacement(replacement) Path
      +remove_abandoned_roots() list
      -_isolated_staging_path(bucket, destination) Path
    }
    class InstallTransaction {
      +commit(result) InstallResult
    }
    class FileLock {
      <<ExternalLock>>
    }
    InstallTransaction *-- ResolutionStagingSession : owns
    ResolutionStagingSession ..> FileLock : activity marker

flowchart TD
    A[download callback] --> B[prepare replacement]
    B --> C[Create .apm-resolution-staging 12-hex replacement root]
    C --> D[Write package content beneath staged path]
    D --> E[publish replacement]
    E --> F[Install transaction commit]
    F --> G[Remove inactive 12-hex or legacy 32-hex staging roots]

Recommendation

Add the nested staged-file Windows path-budget regression test, then ship this bounded portability fix. Track the concise changelog entry alongside it so the user-facing upgrade value is clear.

_{This panel is advisory. Re-apply the panel-review label after addressing feedback to re-run.}

Pushed the nested-payload assertion and the Unreleased Fixed entry the panel asked for in d4a9605. Expressed as headroom the budget is concrete: with a representative nested file beneath the slot, the longest project root that still fits 260 goes from 61 characters to 129, which is exactly the 68 the change frees. Worth flagging that this is headroom and not a long-path guarantee, since the 134-character OneDrive root already used in that test still overflows once package content is appended, even after the fix. On reproducing the original WinError 206, this machine has LongPathsEnabled=1, so a 373-character path creates without complaint and the failure cannot surface here; you would need the Windows default of 0 to see it.

That's great, thanks. I did reproduce it locally in the end and the conclusion was that with LongPathsEnabled=0, the pre-fix staging layout cannot create a realistic nested staged file at 313 characters. This PR layout reduces the same path by 68 characters, to 245 characters, and the write succeeds.

The failed operation reported WinError 3, rather than the issue's WinError 206, because Path.mkdir(parents=True) fails while resolving an overlong intermediate parent. The observable defect is reproduced: the pre-fix layout fails under default MAX_PATH constraints, while the shortened layout succeeds.

So, Sergio Sisternes (@sergio-sisternes-epam) Daniel Meppiel (@danielmeppiel) , I approve this PR fixes the issue with the long path WinError.

APM Review Panel: ship_with_followups

The bounded Windows staging repair is supported by current-head evidence; tighten the changelog claim and keep missing issue scope evidence separate from technical readiness.

panel-mode=full; personas=python-architect,devx-ux-expert,supply-chain-security-expert,oss-growth-hacker,doc-writer,test-coverage-expert,performance-expert,apm-ceo

cc Mohammed Alkindi (@MohammedAlkindi) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

Material delta since the previous panel: at d4a9605, both requested additions are present: the realistic nested-payload path-budget assertion and the Unreleased Fixed entry. The windows_compat marker is also present; the outdated inline marker thread is not a new finding. Luke's September 21 Windows report supplies the reproduction requested in Sergio's latest owner comment: with LongPathsEnabled=0, the old 313-character path fails with WinError 3, while the shortened 245-character path writes successfully. Local exact-head macOS runs passed 51 tests in the two touched test files and 3 staging-recovery integration tests; the Windows-marker subset selected 41 passing tests, not 41 additional cases. This session did not execute on Windows or establish full CI status.

The panel found no demonstrated correctness regression in this bounded change. Existing containment and locking checks remain in place, legacy/current orphan cleanup is covered, and the change adds no filesystem scans or asymptotic cost. The substantive follow-up is release-note precision: 68 characters are saved for hashed replacement/backup slots, while prepare_path's unhashed backup paths save only 20. "Every staged path" and unqualified "no longer fails" are therefore too broad. The regression test itself records that the longer OneDrive checkout still overflows with payload.

Scope evidence remains incomplete. #2896 has status/accepted, but its latest scope comment is still the September 12 withdrawal; no subsequent issue scope record was found, and the neutral eligibility report remains needs-evidence. Sergio's later PR support and the completed Windows reproduction support this advisory review, not an invented issue authorization. Existing contributions are considered on their merits under current policy. A responsible maintainer still needs to record current bounded scope, acceptance criteria, and a willing review contact on the issue. Daniel's stated lack of personal review capacity is unchanged; the existing Daniel/Sergio review requests were preserved, not treated as proof of availability.

Dissent. DevX and Growth regard the changelog wording as polish; the synthesis gives Doc Writer's concrete 68-versus-20 distinction more weight. This does not reopen the already-addressed request to add a changelog.

Aligned with: Secure by default: existing containment and locking checks are retained. Governed by policy: technical advice does not replace human scope evidence. Multi-host support: less Windows staging overhead without requiring users to change corporate OS policy. OSS community-driven: recognize the contributor's bounded fix and supplied reproduction.

Panel summary

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

Top 3 follow-ups

1. [Doc Writer] Describe bounded headroom, not universal success or 68 characters on every staging path. Suggested wording: "apm install gains 68 characters of Windows path headroom for hashed resolution-staging slots, reducing MAX_PATH failures. Deep checkouts can still exceed the limit; cleanup continues to recognize older staging-root names."
2. [Doc Writer] Add by @MohammedAlkindi (#2941) to the entry; retain the issue reference separately if useful. This is attribution polish, not another missing-changelog request.
3. [Supply Chain Security Expert] Optional later hardening: detect an already-reserved slot before deleting its contents and name both destinations. No natural hash collision was demonstrated; ordinary per-session collision risk is extremely low. This is not a request to expand this repair.

Architecture

classDiagram
    direction LR
    class InstallTransaction {
        +resolution ResolutionStagingSession
        +commit(result) InstallResult
        +rollback() None
    }
    class ResolutionStagingSession {
        <<Guard>>
        -_staging_root Path
        -_lock Lock
        +prepare_replacement(path) Path
        +commit() list
        +rollback() list
        +remove_abandoned_roots() list
        -_isolated_staging_path(bucket, dest) Path
    }
    class ensure_path_within {
        <<Pure>>
    }
    class safe_rmtree {
        <<IOBoundary>>
    }
    class _STAGING_NAME {
        <<Constant>>
    }
    InstallTransaction *-- ResolutionStagingSession : owns
    ResolutionStagingSession ..> ensure_path_within : guards paths
    ResolutionStagingSession ..> safe_rmtree : removes trees
    ResolutionStagingSession ..> _STAGING_NAME : cleanup filter
    note for ResolutionStagingSession "Mutex-guarded staging:\nuuid4().hex[:12] root\nsha256()[:16] slot\nfullmatch 12 or 32 cleanup"

flowchart TD
    A["apm install"] --> B["InstallTransaction"]
    B --> C["ResolutionStagingSession"]
    C --> D["12-hex session root"]
    D --> E{"operation"}
    E -->|prepare_replacement| F["_isolated_staging_path"]
    F --> G["16-hex destination slot"]
    G --> H["Create replacement parent"]
    E -->|commit| I["Remove own staging root"]
    I --> J["Release own staging lock"]
    J --> K["InstallTransaction.commit then calls remove_abandoned_roots"]
    K --> L{"12-hex or legacy 32-hex candidate"}
    L -->|matches| M["Attempt activity lock"]
    M -->|acquired| N["Containment-checked cleanup"]
    M -->|busy| O["Keep active session"]
    L -->|no match| P["Keep unrelated path"]
    E -->|rollback| Q["Restore journaled paths"]
    Q --> R["Remove own staging root and release own lock"]

Recommendation

Technically, ship with the bounded changelog follow-ups above. The previous panel's evidence requests are satisfied; no broader Windows redesign, unrelated failure cleanup, or substantive implementation is being requested. Human issue-scope recording remains a separate unresolved governance action, not something this panel or the accepted label can supply. No labels, assignments, reviewer requests, code, or merge state were changed.

python -m pytest -q tests/unit/install/test_resolution_staging_relocate.py tests/unit/install/test_install_transaction.py
51 passed in 8.36s

python -m pytest -q tests/unit/install/test_resolution_staging_relocate.py tests/unit/install/test_install_transaction.py -m windows_compat
41 passed, 10 deselected in 4.69s

python -m pytest -q tests/integration/test_resolution_staging_recovery.py
3 passed in 0.50s

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

Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.