Skip to content

[amplifier-foundation] Failed pinned-SHA resolve leaves poisoned cache that silently serves the wrong commit #347

Description

@bkrabach

Component: amplifier-foundation (Issues disabled on foundation repo; filed here per ecosystem convention)

Summary

When a pinned-SHA git resolve fails on the full-clone fallback path (e.g. a typo'd but valid-looking 40-hex SHA), the cache directory is left behind containing a full clone checked out at the default-branch tip. The next resolve of the same pinned URI hits the cache, passes the integrity check, and silently serves that wrong commit — no error, ever again, until the cache is cleared. A mistyped pin silently becomes "latest", which is the exact failure mode pinning exists to prevent.

Reproduction

  1. Resolve a git URI pinned to a 40-hex SHA that doesn't exist in the repo (e.g. a typo of a real SHA):
    git+https://…/repo@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  2. First resolve: the fast single-commit fetch fails (expected — "reference is not a tree"), the fallback full clone succeeds, git checkout <sha> fails, and BundleNotFoundError propagates with a clear error. So far so good.
  3. But the exception propagates with no cleanup: _clone_at_commit's fallback (amplifier_foundation/sources/git.py:274) does rmtree → full clone → checkout, and a checkout failure leaves cache_path containing a full clone at the default-branch tip.
  4. Second resolve of the same URI: the cache-hit path (git.py:303-315) checks only existence + _verify_clone_integrity. The orphan has .git + the expected structure → integrity passes → the resolve succeeds and serves default-branch-tip content for a URI pinned to a SHA that never existed.

Observed Empirically

Bonus: the orphan has no .amplifier_cache_meta.json, and get_status on the URI still reports "Pinned to (no updates possible)" (git.py:419-423) — so the poisoned cache is also invisible to update checking.

What the Code Already Does (Partial-Fix Neighborhood)

  • The fallback path DOES clean up before its own clone: shutil.rmtree(cache_path, ignore_errors=True) at git.py:274 — but only before the clone, not on checkout failure after it.
  • The cache-hit path DOES self-heal one failure class: _verify_clone_integrity failure triggers rmtree + re-clone (git.py:305-307). Integrity verifies clone structure, not that the checked-out commit matches the requested pin — so a wrong-commit orphan sails through.
  • sale.order: Mostrar Comisión, Costo Envío e Ingreso Neto en tab Financiero Canal #284's retry wrapper (_run_git_network_op, git.py:279) is unrelated: the clone succeeds; it's the checkout that fails, and checkout is not retried (correctly — retrying can't fix a nonexistent SHA).

Impact

Anyone using commit-SHA pinning for reproducibility — eval runs, CI, "same URI, same code" workflows — which is precisely the audience pinning targets. One mistyped pin turns a hard-fail into a silent tracking-latest source on every subsequent resolve from that cache. The failure is silent and persistent: no error, no update-status signal, wrong code served indefinitely.

Fix Shape

Two complementary changes:

  1. Cleanup on checkout failure (the writer): in _clone_at_commit's fallback (git.py:274), wrap the git checkout <sha> so a failure rmtree's cache_path before re-raising. Two lines.
  2. Resolved-commit check on cache hits for SHA refs (the reader): in resolve()'s cache-hit path (git.py:303-315), when parsed.ref is a full commit SHA, verify the cached clone's HEAD equals the pinned SHA; on mismatch, treat as invalid cache (rmtree + re-resolve). This also heals any already-poisoned caches in the wild, which fix (1) alone cannot.

Test shape (composed-state, not branch coverage): one test whose input state is "cache dir left by a failed pin attempt" (full clone at wrong commit, no meta file) → resolve of the pinned URI must NOT serve it.

Environment

Linux aarch64 (Ubuntu, kernel 6.17), Python 3.13.11. Reproduced on amplifier-foundation main @ 49b97b1 (post-#283/#284); bug is pre-existing (present in #282 as merged; #284 did not change it).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions