[BUG] Parallel sparse-checkout race condition when two subdirectory deps reference the same repository

[itayozer9]

Describe the bug

When an apm.yml declares two (or more) virtual subdirectory dependencies that reference the same GitHub repository (e.g. org/repo/shared and org/repo/plugins), the parallel resolution introduced in v0.12.1 causes a race condition. Both dependencies attempt to sparse-checkout the same cloned repo concurrently, but only one subdirectory path is configured — the other always fails with:

Failed to download dependency <org>/<repo>: Subdirectory '<subdir>' not found in repository

Each subdirectory dependency works perfectly in isolation. The failure only occurs when both are declared together and resolved in parallel from a clean state (no cached clone).

To Reproduce

1. Create a GitHub repo (e.g. org/my-apm-packages) with two subdirectory packages, each containing its own apm.yml:

my-apm-packages/
├── apm.yml              # root package
├── shared/
│   ├── apm.yml          # name: my-shared
│   └── .apm/
│       └── skills/
│           └── skill-a/
└── plugins/
    ├── apm.yml          # name: my-plugins
    └── .apm/
        └── skills/
            └── skill-b/

2. In a consumer project, declare both subdirectory deps in apm.yml:

dependencies:
  apm:
  - org/my-apm-packages/shared
  - org/my-apm-packages/plugins

3. Ensure a clean state (no cached clone):

rm -rf apm_modules apm.lock.yaml

4. Run:

apm install --verbose

5. Observe that one dependency installs successfully while the other fails:

[>] Resolving my-apm-packages-shared...
[>] Resolving my-apm-packages-plugins...
  [i] github.com -- token from git-credential-fill
  Failed to download dependency org/my-apm-packages: Subdirectory 'shared' not found in repository

6. Run apm install a second time — it now succeeds because the first run cached the full clone and the second run reuses it:

[+] github.com/org/my-apm-packages/shared @abc1234
[+] github.com/org/my-apm-packages/plugins @abc1234
[*] Installed 2 APM dependencies in 12.1s.

7. Confirm the bug is specific to multi-subdirectory — changing apm.yml to reference only one subdirectory at a time always succeeds on a clean install.

Expected behavior

Both subdirectory dependencies from the same repository should resolve successfully on the first apm install from a clean state. The parallel sparse-checkout dedup logic should either:

• Clone the repo once and then check out both subdirectories sequentially, or
• Serialize sparse-checkout operations for deps sharing the same repo clone

Environment (please complete the following information):

• OS: macOS 15.7.4 (Sequoia)
• Python Version: 3.14.0
• APM Version: 0.12.1 (41ab121)
• Node Version: 24.6.0
• Git Version: 2.49.0

Logs

First install (clean state) — fails:

[>] Installing dependencies from apm.yml...
Parsed apm.yml: 2 APM deps, 0 MCP deps
[>] Resolving my-apm-packages-shared...
[>] Resolving my-apm-packages-plugins...
  [i] github.com -- token from git-credential-fill
  Failed to download dependency org/my-apm-packages: Subdirectory 'shared' not found in repository
Resolved 2 direct dependencies (no transitive)
Phase: resolve -> 10.252s
  Skipping org/my-apm-packages/shared (already failed during resolution)
  [+] github.com/org/my-apm-packages/plugins @abc1234
  |-- 5 skill(s) integrated -> .agents/skills/
  [x] 1 package failed:
    +- my-apm-packages-shared -- Failed to download dependency org/my-apm-packages: Subdirectory 'shared' not found in repository
[!] Installed 1 APM dependency in 22.1s with 1 error(s).

Second install (cached clone) — succeeds:

[>] Installing dependencies from apm.yml...
Parsed apm.yml: 2 APM deps, 0 MCP deps
Using apm.lock.yaml (1 locked dependencies)
[>] Resolving my-apm-packages-plugins...
[>] Resolving my-apm-packages-shared...
  [+] github.com/org/my-apm-packages/shared @abc1234
  |-- 2 skill(s) integrated -> .agents/skills/
  [+] github.com/org/my-apm-packages/plugins @abc1234
  |-- 5 skill(s) integrated -> .agents/skills/
[*] Installed 2 APM dependencies in 12.1s.

Additional context

• This is a regression from APM 0.9.4, which resolved dependencies sequentially and handled this case correctly.
• The issue was likely introduced by the "parallel level-batched BFS dependency resolution" and "in-install repo clone dedup for subdirectory deps" changes in v0.12.1.
• The race is non-deterministic in which subdirectory fails, but in practice the second one to start its sparse-checkout always loses because the first has already configured the sparse-checkout cone for only its own path.
• Workaround: Run apm install twice — the first run populates the clone cache, and the second succeeds. Alternatively, restructure to use a single root-level .apm/ directory so consumers only need one dependency entry per repository.

[danielmeppiel]

Root cause confirmed

This is a regression from #1116's WS2a in-run shared-clone dedup, surfacing only in the multi-subdir-from-same-repo + parallel resolution case. Sequential 0.9.4 worked; the new path doesn't.

The composition failure

Two abstractions don't compose:

• SharedCloneCache.get_or_clone(key, clone_fn) (src/apm_cli/deps/shared_clone_cache.py): runs clone_fn at most once per key (host, owner, repo, ref) and returns the same path to all callers. Correct in isolation.
• _try_sparse_checkout(..., subdir_path, ...) (src/apm_cli/deps/github_downloader.py:1531): mutates the working tree's sparse cone to contain one subdir.

In download_subdirectory_package (github_downloader.py:1676-1707), the closure _shared_clone_fn calls _try_sparse_checkout with the first caller's subdir_path. The cache then hands the same path to all subsequent callers, but the working tree on disk only contains the first caller's subdir. Line 1788 (source_subdir = temp_clone_path / subdir_path) then fails for the others with the exact error in the title.

Why the second apm install succeeds

WS3 (src/apm_cli/cache/git_cache.py) populates a full bare clone on first run; the second run takes the persistent-cache branch (line 1654) and gets a full checkout with all subdirs present.

Why existing tests didn't catch it

tests/unit/deps/test_shared_clone_cache.py::test_two_subdir_deps_share_single_clone calls download_subdirectory_package sequentially, so its mocked clone_fn produces both subdirs on the single shared call. The bug only manifests when two callers race through the lock with different subdir_path closures.

Recommended fix (reviewed with python-architect)

Option A: drop sparse in the shared-cache path; do a full --depth=1 clone. Downstream copy code is unchanged. Trade-off:

• Preserves the dominant perf+ux: comprehensive overhaul of apm install (cache, parallel BFS, UX) #1116 win: one network fetch per repo+ref instead of N.
• Loses the smaller "sparse cone" delta in the multi-subdir case (but --depth=1 already keeps payload small for typical APM packages).
• ~10-line change in _shared_clone_fn; no new parameters; no API surface changes.
• Sparse path stays unchanged for the legacy single-dep code path (no contention there).

We considered:

• B. Bare clone + per-consumer git worktree add (uv-style): principled but ~150 LOC, worktree lifecycle, Windows edge cases. Defer until profiling shows we need it.
• C. Cache-key per subdir: defeats WS2a entirely.
• D. Per-consumer sparse add on shared dir post-lock: mutating shared state under concurrent reads is fundamentally unsafe.
• E. Plumb consumer-count from resolver: over-engineered for marginal sparse savings.

Regression test

A new test test_parallel_different_subdirs_both_succeed in tests/unit/deps/test_shared_clone_cache.py using a threading.Barrier(2) to force true parallelism, plus parallelizing the existing test_two_subdir_deps_share_single_clone.

Lockfile parity

Preserved. Lockfile is SHA-based (repo.head.commit.hexsha); same ref -> same SHA whether sparse or full.

I'll cut a PR with the fix + regression test if this direction looks good.

[itayozer9]

@danielmeppiel Thanks for the thorough analysis - Option A looks great, cleanest trade-off.

One additional data point: apm deps update hits the same race. In my workflow that's actually the more frequent command (updating existing deps rather than fresh installs). Can you confirm the fix covers that code path as well, or does deps update go through a different clone/checkout flow?

[danielmeppiel]

Update: shipping Option B (revised after design review)

After deeper review with python-architect (3 rounds, verdict: PROCEED) plus a 3-specialist panel (auth, supply-chain, test-coverage — all approved), I'm switching from Option A to Option B: bare cache + per-consumer materialization, mirroring WS3's GitCache pattern (src/apm_cli/cache/git_cache.py:209-389).

Why B over A

Option A reverts WS2a to a non-shared --depth=1 full clone per consumer. That fixes the race but regresses #1116's headline win (multi-subdir cold install: 9.5s → 1.77s) back toward 9.5s for the same case. The race is in WS2a's sparse layer; the shared cache layer is sound. B keeps the sharing, fixes the sparseness.

The architectural insight: WS3 already does exactly this for persistent caching — bare clone + git clone --local --shared --no-checkout + checkout. We were inventing a different shape in WS2 (sparse-on-shared) when we could mirror WS3's proven shape with a temp lifecycle.

Architecture (one paragraph)

SharedCloneCache becomes a key→bare-repo-path cache in tempfile.mkdtemp(dir=get_apm_temp_dir()). Cache miss runs _bare_clone_with_fallback (3-tier: tier-1 init+fetch --depth=1 for SHA refs, tier-2 full bare clone, tier-3 error). Each consumer materializes via git clone --local --shared --no-checkout + core.autocrlf=false + LFS smudge disabled + git checkout HEAD. The transport plan (ADO bearer retry + protocol fallback + auth resolution) is extracted from _clone_with_fallback into _execute_transport_plan(repo_url, target, *, dep_ref, clone_action) Strategy template — both working-tree and bare paths compose from it.

apm deps update — same code path

Confirmed @itayozer9: apm deps update for subdir deps flows through download_subdirectory_package → _shared_clone_fn (same callsite at github_downloader.py:1676-1707). Same race, same fix.

Panel findings folded in

• Auth + supply-chain convergent MAJOR: tokenized remote.origin.url persists in bare's .git/config (in temp until end-of-run). Fix: one git remote set-url origin redacted:// after each successful bare clone. Bare is read-only after lock release, so no functional impact.
• Test-coverage: 3 BLOCKER + 4 MAJOR test gaps in original plan — all closed (tier-2 SHA fallback, _wt_action rmtree behavior change, missing-subdir error wording preservation, ADO×SHA composition, LFS smudge disabled, E2E parametrization, update-ref HEAD assertion).
• 4 implementation notes captured (exception type union in transport template, _sanitize_url on subprocess errors, APM_TEMP_DIR permissions doc, deferred bare integrity check rationale).

Surface

~700 lines (~240 src, ~460 test) vs Option A's ~10. Justified because:

1. Option A regresses PR perf+ux: comprehensive overhaul of apm install (cache, parallel BFS, UX) #1116's documented perf win.
2. Long-term, this convergence sets up unifying WS2 with WS3 (deferred §7.2).
3. The transport-plan refactor pays itself back across 8 existing callsites of _clone_with_fallback.

Test coverage (19 scenarios)

Regression (parallel subdirs), lockfile parity (cold WS3 / warm WS3 / WS2), SHA-ref tier-1 + tier-2 fallback, ADO bearer × SHA composition, CRLF determinism, LFS smudge disable assertion, lifetime invariant (rev-parse from bare not consumer), update-ref HEAD assertion, token redaction in bare config, E2E parametrized over (symbolic / sha / default-branch). Windows is implicit via CI matrix (build-release.yml runs unit tests on windows-latest).

Next

Cutting the PR. Will tag this issue on PR open.

[danielmeppiel]

Fix is up in #1135 (now ready for review). Summary: SharedCloneCache (WS2) now stores subdir-agnostic BARE clones keyed only on (host, owner, repo, ref); each consumer materializes its own working tree via git clone --local --shared --no-checkout + git checkout HEAD. Mirrors the WS3 GitCache._create_checkout pattern. Eliminates the parallel sparse-checkout race by removing the shared mutable working-tree state, rather than relying on locks. Includes a regression-trap unit test (TestBareCacheRaceCondition) plus a 3-variant E2E (tests/integration/test_install_subdir_dedup_e2e.py) that drives parallel download_subdirectory_package against two sibling subdirs of the same repo+ref. Apm-review-panel ran on the PR with 0 blocking findings; recommended stance ship_with_followups.