fix(cache): pin core.autocrlf=false on GitCache checkouts

[Sergio Sisternes (sergio-sisternes-epam)]

fix(cache): pin core.autocrlf=false on GitCache checkouts

TL;DR

GitCache._create_checkout is the default path for git-subpath dependencies (owner/repo/<subdir>#ref). It cloned --no-checkout and checked out the SHA without pinning core.autocrlf=false, so a Windows host with the Git for Windows default recorded CRLF package content_hash values that Linux apm install --frozen rejected.

This PR adds -c core.autocrlf=false to the cache-layer git argv (outranks env-frozen host config), persists the same pin on the checkout, and rematerializes unpinned shards. content_hash.py is unchanged.

Note

Scope approval and review contact: danielmeppiel on #2971. Out of scope: hash-algorithm changes, normalizing intentionally committed CRLF, whole-repo (non-subpath) behavior.

Problem (WHY)

• On Windows with system core.autocrlf=true, apm lock / apm install materializes git-subpath trees with CRLF. compute_package_hash() hashes raw bytes, so the lockfile is not portable.
• bare_cache.materialize_from_bare already pinned core.autocrlf=false; GitCache (the path apm install uses by default since persistent cache) did not.
• On the sparse/promisor path, git_network_env freezes host config into GIT_CONFIG_KEY_n. Env config outranks .git/config; only -c wins.

Why these matter: content_hash.py already states that "git-materialized content already does (identical bytes at a pinned commit on every OS)". Host lock-in of lockfile hashes also violates P5 -- Portability over vendor lock-in ("Lock-in of any flavor -- vendor, runtime, host -- is a regression.").

Approach (WHAT)

#	Fix
1	Add -c core.autocrlf=false to _safe_git_args() so every GitCache git subprocess outranks host and env-frozen config.
2	Persist core.autocrlf=false in the consumer .git/config after --no-checkout clone so shards are recognizable.
3	On cache hit / write-dedup, rematerialize shards that lack the pin so existing Windows caches heal without apm cache clean.
4	Docs: lockfile content_hash contract and the content-hash-mismatch troubleshooting note.

Implementation (HOW)

• src/apm_cli/cache/git_cache.py -- Extends the existing _safe_git_args owner (L62-L88). Architecture: ordinary-fix. Did not change content_hash.py, git_env.py snapshot keys, or core.eol.
• tests/unit/cache/test_git_cache_autocrlf.py -- Real-git trap: GIT_CONFIG_SYSTEM with autocrlf=true; full and sparse checkouts; unpinned-shard rematerialize.
• Existing GitCache tests -- Fake cache-hit fixtures now carry the pin; hermetic clone sequences include the extra git config call.
• docs/src/content/docs/reference/lockfile-spec.md, docs/src/content/docs/troubleshooting/common-errors.md, CHANGELOG.md -- Contract and Unreleased note.

Diagrams

Legend: host core.autocrlf=true still exists, but GitCache checkout now pins -c plus local config before writing working-tree bytes.

flowchart LR
  subgraph host [Host]
    A["core.autocrlf=true"]
  end
  subgraph cache [GitCache._create_checkout]
    B["clone --no-checkout"]
    C["git config core.autocrlf false"]
    D["git -c core.autocrlf=false checkout SHA"]
  end
  subgraph hash [compute_package_hash]
    E["raw bytes SHA-256"]
  end
  A --> B
  B --> C
  C --> D
  D --> E
  classDef new stroke-dasharray: 5 5
  class C,D new

Loading

Trade-offs

• Checkout pin, not hash normalization. Chose to keep raw-byte content_hash. Rejected CRLF-to-LF hashing because the approval record forbids hash-algorithm changes and because committed CRLF must stay visible.
• -c plus persisted config. Persist alone loses to GIT_CONFIG_KEY_n on the sparse path; -c alone cannot mark pre-fix shards. Both are required.
• Did not add core.eol=lf. A repo with * text=auto can still check out native EOL under autocrlf=false. That is a separate host knob; left as a follow-up.
• Did not narrow _materialize_git_config_snapshot. Wider git_env.py change; out of this issue's scope.

Benefits

1. Same pinned git-subpath commit produces the same content_hash under system core.autocrlf=true and on Linux/macOS.
2. apm install --frozen can replay a lockfile generated on either OS.
3. Existing Windows cache shards rematerialize on the next install without apm cache clean.
4. Intentionally committed CRLF bytes remain CRLF.

Validation

Lint (CI-mirror): ruff check, ruff format --check, pylint R0801, scripts/lint-auth-signals.sh all silent / exit 0.

Mutation-break: removing -c core.autocrlf=false from _safe_git_args made test_sparse_checkout_keeps_lf_when_env_freezes_autocrlf_true fail (\r at index 3). Guard restored.

pytest (git cache + new trap)

uv run --extra dev pytest -q tests/unit/cache/test_git_cache_autocrlf.py \
  tests/unit/cache/test_git_cache.py tests/unit/cache/test_git_cache_hardening.py \
  tests/integration/test_git_cache_hermetic.py
85 passed in 1.38s

uv run --extra dev pytest -q tests/unit/cache/test_git_cache.py \
  tests/unit/cache/test_git_cache_hardening.py tests/unit/cache/test_git_cache_recency.py \
  tests/unit/cache/test_git_cache_sparse.py tests/unit/test_git_cache_phase3w4.py \
  tests/unit/test_git_cache_branch_coverage.py tests/integration/test_git_cache_hermetic.py \
  tests/integration/test_git_cache_recency_lifecycle.py \
  tests/unit/cache/test_git_cache_autocrlf.py tests/unit/deps/test_shared_clone_cache.py
250 passed, 2 skipped in 18.27s

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	Lock a git-subpath skill on a host whose system gitconfig sets core.autocrlf=true; the package content_hash matches the LF tree.	Portability by manifest; Governed by policy	tests/unit/cache/test_git_cache_autocrlf.py::test_full_checkout_keeps_lf_under_system_autocrlf_true	unit
2	Sparse git-subpath checkout still stays LF when host autocrlf is frozen into GIT_CONFIG_KEY_n.	Portability by manifest	tests/unit/cache/test_git_cache_autocrlf.py::test_sparse_checkout_keeps_lf_when_env_freezes_autocrlf_true	unit
3	An older Windows cache shard without the pin is rematerialized on the next install.	DevX (pragmatic as npm)	tests/unit/cache/test_git_cache_autocrlf.py::test_cache_hit_rematerializes_unpinned_crlf_shard	unit

How to test

• uv run --extra dev pytest -xvs tests/unit/cache/test_git_cache_autocrlf.py — all four tests pass.
• Point GIT_CONFIG_SYSTEM at a file with core.autocrlf=true, lock a git-subpath dep, and confirm SKILL.md bytes are LF (\n only).
• Confirm content_hash.py is untouched and a file committed with CRLF still hashes as CRLF.
• Review contact is Daniel Meppiel (@danielmeppiel) (do not treat this worker as reviewer).

Closes #2971

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

[Copilot]

🟡 Changes recommended

Unresolved cache-healing concurrency and validation issues, along with documentation and scope corrections, block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR makes GitCache checkout content more consistent across hosts by pinning core.autocrlf=false, healing legacy shards, and updating tests and documentation.

Changes:

• Pins Git checkout configuration and rematerializes unpinned cache shards.
• Adds real-Git regression coverage.
• Updates lockfile guidance, troubleshooting documentation, and the changelog.

File summaries

File	Summary
tests/unit/cache/test_git_cache.py	Updates cache-hit fixtures.
tests/unit/cache/test_git_cache_recency.py	Updates recency fixtures.
tests/unit/cache/test_git_cache_hardening.py	Updates hardening fixtures.
tests/unit/cache/test_git_cache_autocrlf.py	Adds cross-platform autocrlf regression tests.
tests/integration/test_git_cache_hermetic.py	Updates checkout command expectations.
src/apm_cli/cache/git_cache.py	Adds autocrlf pinning and legacy-shard healing.
docs/src/content/docs/troubleshooting/common-errors.md	Updates hash-mismatch guidance.
docs/src/content/docs/reference/lockfile-spec.md	Documents Git checkout hash behavior.
CHANGELOG.md	Adds the unreleased fix entry.

Review details

Suppressed comments (8)

docs/src/content/docs/reference/lockfile-spec.md:220

• This says Git checkouts generally, but the changed pin is in GitCache and the approved scope explicitly excludes whole-repo materialization. Also, core.autocrlf=false does not override repository .gitattributes/core.eol choices, which this PR leaves as a follow-up. Narrow the contract to the GitCache-backed git-subpath path and state that limitation, otherwise the lockfile spec promises cross-OS hashes for paths this PR does not fix.

| `content_hash` | string | no | SHA-256 of the materialized package tree, computed from sorted relative paths and raw file bytes. Git checkouts pin `core.autocrlf=false` so LF-committed content hashes the same on every OS; bytes that are committed as CRLF stay CRLF. For remote dependencies it verifies that downloaded or cached content still matches the lock; for local path dependencies it detects source-tree changes. |

src/apm_cli/cache/git_cache.py:670

• If eviction fails, robust_rmtree is called with ignore_errors=True, so this path still proceeds with the old final_dir. atomic_land can then report a loser and the existing fallback only verifies the SHA, allowing the unpinned checkout to be returned without healing it. Fail closed when final_dir remains after eviction, or make the fallback require the autocrlf pin as well.

                self._evict_checkout(final_dir)

src/apm_cli/cache/git_cache.py:106

• This substring search does not establish that the core.autocrlf key is set to false: a remote URL or comment containing autocrlf=false would make an old CRLF shard look healed and skip rematerialization. Parse the local [core] key (or query git config --local --get --type=bool) and compare its normalized value.

    return "autocrlf = false" in text or "autocrlf=false" in text

src/apm_cli/cache/git_cache.py:89

• This helper is used for every GitCache checkout, and _create_checkout persists and recognizes the pin regardless of sparse_paths; whole-repository downloads call this cache with no sparse paths. That changes and migrates whole-repo cache shards even though the PR description and the approved issue scope list whole-repo behavior as out of scope. Please either constrain the migration/pin to subpath variants or update the approved scope and coverage to include this behavior.

        "-c",
        "submodule.recurse=false",
        "-c",
        "core.autocrlf=false",
    ]

src/apm_cli/cache/git_cache.py:105

• A corrupted or non-UTF-8 .git/config makes read_text raise UnicodeDecodeError, which is not an OSError; the cache-hit path then aborts instead of treating the pin as missing and rematerializing, contrary to the defensive false result intended here. Catch UnicodeError as well.

    try:
        text = config.read_text(encoding="utf-8")
    except OSError:
        return False

tests/unit/cache/test_git_cache_autocrlf.py:26

• This real-Git fixture bypasses the repository's trusted executable resolver and launches a literal git command. Because these tests run in the Windows compatibility gate, use get_git_executable() here so the fixture exercises the same PATH-independent binary selection as GitCache.

        ["git", *args],

tests/unit/cache/test_git_cache_autocrlf.py:17

• This new module runs real Git subprocesses and writes checkout trees, so it belongs to the repository's component behavioral boundary, but it declares only the scheduling windows_compat markers. Add a module-level component marker so the taxonomy inventory classifies every collected node consistently.

import pytest

from apm_cli.cache.git_cache import GitCache, _safe_git_args
from apm_cli.utils.content_hash import compute_package_hash

tests/unit/cache/test_git_cache_autocrlf.py:39

• _neutral_git_env leaves inherited GIT_CONFIG_NOSYSTEM and GIT_CONFIG_PARAMETERS in place. If either is set by the runner, the synthetic system config from _host_autocrlf_true_env can be ignored or overridden, allowing this regression to pass without exercising the host-config failure. Clear both in the neutral environment.

    env["GIT_CONFIG_GLOBAL"] = os.devnull
    env["GIT_CONFIG_SYSTEM"] = os.devnull
    env.pop("GIT_CONFIG_COUNT", None)

• Files reviewed: 9/9 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_with_followups

GitCache now pins host autocrlf conversion; disposable fixture-precondition failures and missing frozen-replay proof warrant focused test and recovery-doc follow-ups.

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

At exact reviewed head 9a5fa1caa68077a8012b216bbfe2c7efb46aa075, all nine returns support the bounded production design. Invocation protection and a persisted migration marker belong in the canonical GitCache owner; raw hashing, hook/submodule safeguards, and auth/transport boundaries remain intact. Full and sparse variants both inherit the fix: acknowledge that shared-owner effect rather than fork checkout policy. The five earlier visible thread topics are addressed. Of eight suppressed concerns, six are fixed, the whole-repository concern establishes no separate production defect, and inherited fixture-environment isolation remains incomplete. The legacy-lock explanation is corrected conceptually; command accuracy is a separate remaining issue. Under P5/P6, no reachable production correctness, architecture, auth, or security-integrity regression was identified. Neither a general config parser nor deferred core.eol/snapshot redesign is warranted.

The strongest remaining evidence is a reproducible fixture defect. In the disposable local probe .review-pr2982-tests/test_review_probes.py::test_host_fixture_actually_exposes_autocrlf_true, assert observed == [(0, "true", False)] failed in four inherited-override cases even though the existing full/sparse checkout tests passed. Six controls/production probes passed, including intentional-CRLF preservation and fail-closed eviction/publication checks. These are fixture-precondition failures, not committed-suite or production-behavior failures; the scratch probe source was removed. Separately, the test-tree audit found no real git-subpath CLI lock/frozen roundtrip across opposite host-autocrlf settings. That leaves the issue's frozen-replay done criterion unproven, not disproven. These signals outweigh the optional subprocess optimization and presentation nits.

Hosted exact-head evidence reports 17 successful checks, including Linux build/tests, lint and Windows checks in CI, plus the docs build; two checks are neutral and docs deployment is skipped. The focused local run passed 114 tests in 21.01s on macOS only after dependency-sync and child-import problems were worked around using a disposable environment, an offline editable checkout, and existing Homebrew packages. No dependency manifests or lockfiles changed. This is a focused fallback-environment pass, not full local CI or local Windows execution. The older author-reported 85/250 counts and mutation result were not reproduced. Repository approval requirements remain independent; this advisory supplies no human sign-off.

Dissent. Growth's assessment that legacy recovery is actionable yields to Doc Writer and DevX's source-backed command analysis: explaining lock regeneration does not make bare apm lock effective. Performance's extra-process reduction remains optional; its synthetic measurement is not an install benchmark and does not justify expanding this fix.

Aligned with: Portable by manifest, Secure by default, Pragmatic as npm, OSS community-driven

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	0	The bounded GitCache fix preserves canonical ownership and safe publication. One fixture-environment concern remains; no reachable production architecture defect found.
CLI Logging Expert	0	0	1	Default output stays quiet; URLs and new failure diagnostics use canonical redaction. One duplicate-log nit.
DevX UX Expert	0	1	0	The bounded checkout fix preserves raw-hash semantics. Legacy-lock recovery guidance still needs an explicit update path and fresh-materialization prerequisite.
Supply Chain Security	0	1	0	Cache integrity remains fail-closed; one inherited-environment regression-fixture gap remains.
OSS Growth Hacker	0	0	1	Legacy-lock recovery is actionable; one release-note claim could match the documented portability boundary more precisely.
Auth Expert	0	1	0	No auth transport or credential-handling regression found. The real-Git fixture still incompletely clears inherited configuration.
Doc Writer	0	2	1	Current docs resolve the earlier scope overclaim; correct the bare apm lock recovery command and refresh the PR body's promises and evidence.
Test Coverage	0	3	0	114 focused tests pass. The hostile-config fixture still leaks inherited overrides; frozen host-switch replay and failed-healing regression traps remain worth adding.
Performance Expert	0	1	0	Warm-hit complexity remains bounded. One optional cold-path improvement removes a measured Git spawn per new or rematerialized checkout.

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

Top 3 follow-ups

1. [Test Coverage] Fix fixture isolation at tests/unit/cache/test_git_cache_autocrlf.py:90 and the other three checkout setups: explicitly remove inherited GIT_CONFIG_NOSYSTEM and GIT_CONFIG_PARAMETERS, then assert effective host autocrlf=true before exercising GitCache. -- Four disposable precondition assertions failed while the shipped checkout tests passed. Keep the correction test-local and retain the existing byte/hash assertions; no production environment-policy change is needed.
2. [Test Coverage] Reuse tests/integration/test_git_cache_recency_lifecycle.py for one parameterized pinned-LF git-subpath scenario: generate a lock under each system-autocrlf setting, remove owned consumer/cache materializations, and replay with --frozen under the opposite setting. -- Assert successful replay, unchanged lock bytes, and the locked raw package hash in both directions. Existing lifecycle coverage does not exercise this specific portability criterion. This closes a proof gap without inventing a frozen-install failure or expanding into a broad matrix.
3. [Doc Writer] Correct docs/src/content/docs/troubleshooting/common-errors.md:90 and verify the documented recovery sequence: require a fresh consumer workspace or rematerialization of the affected installed package, then explicit apm lock --update or apm install --update; warn that mutable refs can advance and the SHA/hash diff needs review. -- Source analysis, not an executed CLI reproduction, shows that bare apm lock retains the existing hash check. A matching same-SHA CRLF apm_modules copy can also be reused even with --update. Healing GitCache alone does not regenerate a legacy project lock.

Architecture

Class and component diagrams

classDiagram
    direction LR
    class GitHubPackageDownloader {
      +download_subdirectory_package()
      +download_package()
      -_persistent_cache_checkout()
    }
    class GitCache {
      <<CacheAside>>
      +get_checkout() Path
      -_ensure_bare_repo()
      -_create_checkout() Path
      -_finalize_sparse_checkout() Path
    }
    class CheckoutPolicy {
      <<Module>>
      _safe_git_args()
      _checkout_pins_autocrlf_false()
    }
    class Integrity {
      <<Module>>
      verify_checkout_sha()
    }
    class CacheLocking {
      <<Module>>
      shard_lock() FileLock
      stage_path() Path
      atomic_land() bool
    }
    class FileLock {
      <<Mutex>>
    }
    class GitEnvironment {
      <<Module>>
      get_git_executable()
      git_subprocess_env()
      git_network_env()
    }
    class GitSparse {
      <<Module>>
      apply_sparse_cone()
      repair_dangling_cone_symlinks()
    }
    GitHubPackageDownloader --> GitCache : persistent checkout
    GitCache ..> CheckoutPolicy : invocation and migration pin
    GitCache ..> Integrity : HEAD verification
    GitCache ..> CacheLocking : stage and publish
    CacheLocking ..> FileLock : creates and acquires
    GitCache ..> GitEnvironment : execution environment
    GitCache ..> GitSparse : configure and repair cone
    note for GitCache "Cache-aside: reuse validated shards; rebuild misses and legacy shards"
    note for CheckoutPolicy "Functions in cache/git_cache.py; not a new runtime class"
    class GitCache:::touched
    class CheckoutPolicy:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    CLI["apm install<br/>commands/install.py: install"] --> SUB["deps/github_downloader.py:<br/>download_subdirectory_package"]
    CLI --> FULL["deps/github_downloader.py:<br/>download_package"]
    SUB --> ENTRY["_persistent_cache_checkout"]
    FULL --> ENTRY
    ENTRY --> GET["cache/git_cache.py: GitCache.get_checkout<br/>_resolve_sha and _variant_key"]
    GET --> HIT{"[I/O] Non-refresh hit:<br/>verify_checkout_sha AND<br/>_checkout_pins_autocrlf_false?"}
    HIT -->|yes| REUSE["[LOCK] shard_lock<br/>_finalize_sparse_checkout<br/>[I/O] _record_checkout_access"]
    REUSE --> RETURN["Return checkout Path to downloader"]
    HIT -->|no| LEGACY["Retain SHA-valid unpinned tree;<br/>[FS] evict SHA-invalid hit"]
    LEGACY --> BARE["[LOCK] [NET] [EXEC]<br/>_ensure_bare_repo"]
    BARE --> CREATE["[LOCK] _create_checkout:<br/>shard_lock before staging"]
    CREATE --> DEDUP{"[I/O] Locked re-probe:<br/>SHA and local pin valid?"}
    DEDUP -->|yes| REUSE
    DEDUP -->|no| OLD{"SHA-valid unpinned final_dir?"}
    OLD -->|yes| EVICT["[FS] _evict_checkout"]
    EVICT --> REMAINS{"[I/O] final_dir still exists?"}
    REMAINS -->|yes| ERROR["Raise RuntimeError;<br/>do not return old shard"]
    REMAINS -->|no| STAGE["[FS] stage_path beside final_dir"]
    OLD -->|no| STAGE
    STAGE --> CLONE["[EXEC] clone --local --shared --no-checkout<br/>_safe_git_args includes core.autocrlf=false"]
    CLONE --> PIN["[EXEC] Persist config core.autocrlf false<br/>before materializing working-tree bytes"]
    PIN --> SPARSE{"sparse_paths / promisor consumer?"}
    SPARSE -->|yes| CONE["[EXEC] git_network_env<br/>apply_sparse_cone with _safe_git_args"]
    SPARSE -->|no| CHECKOUT["[EXEC] checkout SHA<br/>-c core.autocrlf=false outranks frozen config"]
    CONE --> CHECKOUT
    CHECKOUT --> FINALIZE["[I/O] _finalize_sparse_checkout<br/>[EXEC] repair_dangling_cone_symlinks if needed"]
    FINALIZE --> LAND{"[FS] cache/locking.py:<br/>atomic_land succeeded?"}
    LAND -->|yes| RETURN
    LAND -->|no| WINNER{"[I/O] Winner SHA and pin valid?"}
    WINNER -->|yes| RETURN
    WINNER -->|no| FAILURE["[FS] _evict_checkout<br/>raise RuntimeError"]

Loading

Recommendation

Source-correctness review supports shipping the bounded implementation. Prefer folding the small fixture-isolation and recovery-doc corrections into this PR, and keep the single host-switch frozen replay explicitly unfinished until executable evidence establishes the issue's done criterion. Do not describe the current tests as proving that roundtrip, or inflate their absence into a demonstrated runtime defect. The maintainer and author retain the shipping decision and independent formal review.

---

Full per-persona findings

Python Architect

• [recommended] Apply fixture environment deletions, not just the remaining key/value pairs at tests/unit/cache/test_git_cache_autocrlf.py:90
  _neutral_git_env removes GIT_CONFIG_NOSYSTEM and GIT_CONFIG_PARAMETERS from its returned copy, but the four per-test setenv loops do not remove those keys from os.environ. Their subsequent deletion loops only handle COUNT/KEY/VALUE entries. The synthetic system autocrlf=true configuration can therefore remain disabled or overridden. I reproduced the current fixture setup with real Git: inherited NOSYSTEM=1 left autocrlf unset, and inherited PARAMETERS='core.autocrlf=false' selected false; explicitly removing both restored true. tests/conftest.py does not clear these variables. This weakens the regression trap rather than demonstrating a production failure.

  Design patterns

  • Used in this PR: Cache-aside -- GitCache reuses valid shards and rebuilds legacy shards through its existing locked publication path.
  • Pragmatic suggestion: none -- retain the bounded production design; consolidate the repeated environment application into one small fixture helper.
    Suggested: Explicitly monkeypatch.delenv both variables before invoking GitCache in all four real-checkout cases. Share that setup locally, and assert that the effective pre-checkout host configuration actually reports autocrlf=true.
    Proof (manual, manual-only): tests/unit/cache/test_git_cache_autocrlf.py::test_full_checkout_keeps_lf_under_system_autocrlf_true -- proves: The host-autocrlf regression can assert LF bytes without establishing the intended system autocrlf=true precondition. [portability-by-manifest]
    assert skill.read_bytes() == _LF_BODY

CLI Logging Expert

• [nit] Emit the rematerialization message once. at src/apm_cli/cache/git_cache.py:254
  An unpinned hit logs here, then again at line 683 under lock. Only verbose/INFO output is affected; default WARNING remains quiet.
  Suggested: Keep the locked emission; remove the preliminary one.

DevX UX Expert

• [recommended] Bare apm lock does not reliably regenerate a legacy CRLF hash at docs/src/content/docs/troubleshooting/common-errors.md:90
  The documented recovery must actually escape the mismatch. commands/lock.py defaults update_refs to false and forwards it unchanged. FreshDependencySource.acquire in install/sources.py:814-840 rejects a freshly downloaded LF hash against the existing CRLF hash unless update_refs or a separately authorized hash-change condition applies; lockfile_only does not bypass this check. Consequently, bare apm lock can reproduce the same error. An existing CRLF apm_modules copy can also be reused when its hash matches the old lock, including under --update when the resolved SHA is unchanged (install/phases/integrate.py:123-146). Healing GitCache shards is not equivalent to regenerating the project lock.
  Suggested: Document recovery from a fresh consumer workspace, or explicitly require rematerializing the affected installed package, then use apm lock --update or apm install --update. Explain that --update can advance mutable refs and require reviewing the SHA/hash diff before committing. Keep normal and frozen integrity validation unchanged.

Supply Chain Security

• [recommended] Remove inherited config overrides when applying the test environment at tests/unit/cache/test_git_cache_autocrlf.py:90
  _neutral_git_env removes GIT_CONFIG_NOSYSTEM and GIT_CONFIG_PARAMETERS only from its copy. The setup loops merge that copy into os.environ without deleting those inherited keys. An isolated execution of the actual setup followed by real Git config lookup observed autocrlf=true with clean input, no value with NOSYSTEM=1, and false with PARAMETERS overriding autocrlf. The regression can therefore miss its intended hostile-host precondition. This is a validation gap, not a production security bypass.
  Suggested: Explicitly monkeypatch.delenv both keys in all four checkout setups, alongside the existing COUNT/KEY/VALUE cleanup; assert the synthetic host config is effective before exercising GitCache.
  Proof (manual, manual-only): tests/unit/cache/test_git_cache_autocrlf.py -- proves: The host-autocrlf regression can run without its required core.autocrlf=true precondition. [portability-by-manifest,secure-by-default]

OSS Growth Hacker

• [nit] Describe the autocrlf fix rather than unconditional cross-platform hash equality. at CHANGELOG.md:29
  The lockfile specification correctly preserves .gitattributes/core.eol exceptions. LF-committed content alone does not establish the broader equality promised here.
  Suggested: Replace the equality clause with 'prevent host autocrlf settings from rewriting LF-committed package content'. No additional EOL policy is needed.

Auth Expert

• [recommended] Remove inherited NOSYSTEM and PARAMETERS when applying the fixture environment at tests/unit/cache/test_git_cache_autocrlf.py:114
  _neutral_git_env removes both keys from its returned copy, but the tests only set entries from that copy. Existing os.environ values survive; subsequent cleanup removes only COUNT/KEY/VALUE entries. A standalone real-Git probe reproduced effective autocrlf=true with clean setup, unset with inherited GIT_CONFIG_NOSYSTEM=1, and false with inherited GIT_CONFIG_PARAMETERS specifying false. Thus the regression can pass without exercising the intended host autocrlf=true condition. This is a fixture-isolation issue, not an application auth bypass.
  Suggested: Explicitly monkeypatch.delenv both variables when applying host_env in all four real-checkout tests. Assert the effective host autocrlf value is true before materialization. Keep this cleanup test-local; production intentionally preserves user configuration.
  Proof (manual, manual-only): tests/unit/cache/test_git_cache_autocrlf.py::test_sparse_checkout_keeps_lf_when_env_freezes_autocrlf_true -- proves: The sparse-checkout regression's host-autocrlf precondition is not guaranteed under inherited runner configuration. [portability-by-manifest]

Doc Writer

• [recommended] Bare apm lock does not repair an existing CRLF content_hash at docs/src/content/docs/troubleshooting/common-errors.md:90
  The new recovery instruction offers apm lock alongside apm install --update, but they are not equivalent. Bare lock defaults update_refs to false and forwards it unchanged into the install pipeline (commands/lock.py:106-110,228-237). The pipeline loads the existing lockfile, and fresh materialization still rejects a different hash when update_refs is false (install/sources.py:814-839). Thus a clean checkout with the legacy CRLF lock can encounter the same mismatch instead of recording the corrected LF hash. This is a documentation command error, not a request to relax integrity verification.
  Suggested: Remove bare apm lock from the recovery parenthetical and retain apm install --update. If retaining a lock-only alternative for fresh materialization, use apm lock --update; note that update can advance mutable refs.

• [recommended] Align PR-body scope and benefits with the qualified documentation
  PR body sections 'TL;DR' and 'Benefits' still describe whole-repo behavior as excluded and promise replay of a lockfile generated on either OS without the current documentation's qualifications. The shared GitCache owner also serves regular whole-repo downloads (github_downloader.py:1895-1910), so those cache variants receive the same pin and rematerialization behavior. Separately, existing CRLF locks require regeneration, and the pin does not override .gitattributes or core.eol. The body should distinguish the bounded subpath objective from this shared-owner effect rather than imply whole-repo behavior is untouched.
  Suggested: Add one sentence acknowledging that full GitCache variants inherit the shared pin. Qualify Benefits 1-2 to the targeted autocrlf conversion and newly generated or regenerated locks, referencing the existing EOL limitation. No cache fork or broader configuration framework is needed.

• [nit] Refresh the PR body's test checklist and evidence labels
  PR body 'How to test' still says four tests, while the current regression module contains seven. 'Scenario Evidence' labels the real-checkout cases as unit even though they use real Git and filesystem I/O and now carry the component marker. 'Implementation' links the initial 8f1693b revision, and Validation's 85/250 results and mutation claim are earlier author-reported evidence rather than current local results.
  Suggested: Change four to seven, label the real-checkout scenarios component/integration-with-fixtures, refresh implementation permalinks, and identify the older validation results by revision instead of presenting them as exact-head evidence.

Test Coverage

• [recommended] Apply the neutral environment's deletions before certifying the hostile-host regression. at tests/unit/cache/test_git_cache_autocrlf.py:90
  neutral_git_env removes NOSYSTEM/PARAMETERS from its copy, but lines 90-95 only set surviving keys and delete COUNT/KEY/VALUE_. Inherited NOSYSTEM/PARAMETERS remain in os.environ. Four real-Git probes called the existing full/sparse tests: both tests passed with NOSYSTEM=1 while effective autocrlf was unset, and with PARAMETERS='core.autocrlf=false' while it was false. Clean controls observed true and passed. This is a reproducible false-positive regression fixture, not a production checkout defect; the earlier suppressed concern is only partially fixed.
  Suggested: Explicitly monkeypatch.delenv both inherited keys when applying host_env in these tests, not merely in the copied dictionary. Assert Git sees the intended hostile autocrlf=true before the cache call; keep the existing byte/hash assertions.
  Proof (failed, integration-with-fixtures): .review-pr2982-tests/test_review_probes.py::test_host_fixture_actually_exposes_autocrlf_true (disposable local probe; removed after execution) -- proves: The full/sparse LF checkout regression must actually exercise the hostile host setting before certifying portable package hashes. [portability-by-manifest,devx]
  assert observed == [(0, "true", False)]

• [recommended] Defend the scoped frozen lockfile replay promise through a real git-subpath consumer. at tests/unit/cache/test_git_cache_autocrlf.py:100
  The executed full/sparse tests are integration-with-fixtures evidence despite their unit directory, but stop at GitCache and raw hashing: none writes or replays an APM lockfile. Searched tests/**/*.py for autocrlf/GIT_CONFIG_SYSTEM and read the frozen/roundtrip candidates. test_git_cache_recency_lifecycle covers frozen reuse without switching host config; test_virtual_claude_skill_lock_convergence disables GitCache; test_install_content_hash_roundtrip mocks raw download and tests synthetic manifests, not this checkout path. Thus the approved either-platform frozen replay criterion has no matching automated consumer scenario. This is a proof gap, not evidence that frozen installs fail.
  Suggested: Reuse the existing frozen-rehydrate lifecycle fixture for one pinned LF git-subpath scenario: generate the lock under each system-autocrlf setting, remove owned materializations/cache, then install --frozen under the opposite setting. Assert success, unchanged lock bytes, and the same raw package hash. No additional unrelated lifecycle matrix is needed.
  Proof (missing, e2e): tests/integration/test_git_cache_recency_lifecycle.py::test_frozen_git_subpath_replays_across_host_autocrlf -- proves: apm install --frozen can replay a git-subpath lockfile generated under either host autocrlf setting without changing its hash. [portability-by-manifest,devx]
  assert replay.returncode == 0 assert lock_after == lock_before assert compute_package_hash(materialized_package) == locked_hash

• [recommended] Retain a failure-path trap for a SHA-valid unpinned checkout that cannot be removed. at src/apm_cli/cache/git_cache.py:690
  Current code correctly fails closed, and a disposable real-checkout fault-injection probe passed. However, tests-tree searches for rematerialization/eviction/atomic-land and reads of the matching tests found only successful healing, eviction-helper error swallowing, and loser tests with either a pinned winner or an invalid SHA. No committed test exercises an unremovable SHA-valid unpinned shard. That leaves the recently repaired cache-healing failure behavior without a durable regression trap; this is not a remaining production bug.
  Suggested: Extend the existing real-Git healing fixture with a no-op deletion fault and assert get_checkout raises rather than returns the poisoned path. The existing atomic-land test can additionally cover a valid SHA with a missing pin, without introducing real concurrency machinery.
  Proof (missing, integration-with-fixtures): tests/unit/cache/test_git_cache_autocrlf.py::test_unremovable_unpinned_checkout_is_not_returned -- proves: An older poisoned cache shard is rebuilt or rejected, never silently returned after healing fails. [portability-by-manifest,devx]
  with pytest.raises(RuntimeError, match="Failed to rematerialize unpinned git checkout"): cache.get_checkout(url, sha, locked_sha=sha)

Performance Expert

• [recommended] Persist the pin through the existing clone command instead of spawning Git again at src/apm_cli/cache/git_cache.py:745
  The new setter adds one process for every newly constructed checkout variant: N additional processes across N cold or rematerialized variants. The adjacent clone already persists promisor configuration using clone-level -c options. On this macOS host, 20 isolated config writes had a 22.875 ms median, with substantial variance; this is process-cost evidence, not an install-wall-time estimate. Removing this call is a bounded improvement, not a correctness requirement.
  Suggested: Add clone-level '-c core.autocrlf=false' alongside the existing clone options and remove the separate setter. Retain invocation-level _safe_git_args() protection against frozen config. Preserve the real-Git regressions and assert construction subprocess counts at N and 10*N checkouts rather than adding brittle timing thresholds.
  Proof (manual, manual-only): -- proves: The persisted checkout marker and invocation-level conversion protection can coexist without a separate configuration-writing process.

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

[Sergio Sisternes (sergio-sisternes-epam)]

Prefer folding the small fixture-isolation and recovery-doc corrections into this PR, and keep the single host-switch frozen replay explicitly unfinished

Folded in 78f42b3:

• _apply_hostile_host_env deletes inherited GIT_CONFIG_NOSYSTEM / GIT_CONFIG_PARAMETERS and asserts system core.autocrlf=true before GitCache
• Recovery docs now require rematerializing then apm install --update or apm lock --update; bare apm lock does not rewrite a CRLF hash
• Rematerialize log emits once, under shard_lock
• test_unremovable_unpinned_checkout_is_not_returned fails closed
• Changelog wording no longer claims unconditional cross-OS hash equality

Left unfinished, as recommended: host-switch frozen-replay e2e until that scenario has executable evidence. Also deferred the extra clone-process reduction.