quilt3: recover from corrupt cached manifests, explain unverifiable ones

[QuiltSimon]

Two follow-ups to the unconditional manifest verification (#5184), from tonight's hardening review of master vs 26.7.4.

Fixes

1. Corrupt cached manifest is no longer fatal forever. _from_path ran outside the try, so a cached manifest that failed to parse was never evicted — and nothing re-downloads a file that exists, so one truncated cache entry (exactly what the shared-.tmp race fixed in Verify browsed package manifest hashes #5184 could leave behind) failed every future browse() of that revision with a raw JSON error. Recovery was manually deleting ~/.cache/quilt. Now: evict, re-download once, re-parse; a fresh copy that is also bad raises as-is and is never promoted into the cache.
2. Unverifiable manifests fail with an actionable error. An entry lacking hash/size (typically a non-quilt3 producer) made _get_top_hash_parts raise before the mismatch branch could speak — naming a bare physical key, nothing about manifests, hashes, or what to do. Since the check cannot be skipped, that message is the user's only lead. Now PackageException names the package, the revision, and the failing step, and suggests re-pushing with quilt3.

Verification

• tests/integration/test_packages.py: 230 passed, 1 xfailed.
• Both new tests fail against the unfixed source (all 4 across the V1/V2 registry classes).

Deliberately not fixed here

The review also found that the accepted-parent set from #5180 is monotonic: an object that pushed rev2 and rev3 still accepts rev1, so a stale notebook can silently undo a Package.rollback that the old _origin check would have refused. A per-destination compare-and-swap would fix it without breaking the #2722 mirror flow the set exists for — but that changes semantics #5180 chose deliberately, so it's flagged for its author rather than redesigned here.

🤖 Generated with Claude Code

Greptile Summary

The PR makes remote package browsing recover from an unparsable cached manifest and adds contextual guidance when manifest verification cannot compute a top hash.

• Evicts a failed cache hit, downloads once to a unique temporary path, reparses it, and only then promotes the manifest.
• Wraps missing entry hash or size failures with package, revision, verification, and remediation context.
• Adds integration coverage for cache recovery and unverifiable manifests and documents both changes.

Confidence Score: 4/5

The implementation appears safe to merge, with only the non-blocking changelog links needing correction.

Cache recovery remains limited to cache hits, retries only once through a unique temporary file, and preserves unconditional top-hash verification; the sole accepted issue is incorrect pull-request attribution in the changelog.

Files Needing Attention: docs/CHANGELOG.md

Important Files Changed

Filename	Overview
api/python/quilt3/packages.py	Adds one-shot recovery for unparsable cached manifests and contextual handling for manifests whose entries lack hash or size.
api/python/tests/integration/test_packages.py	Adds integration tests covering successful cache repair and actionable failure without cache promotion.
docs/CHANGELOG.md	Documents both fixes, but the new entries link to PR #5217 rather than PR #5218.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Resolve remote manifest] --> B{Cached manifest exists?}
    B -->|No| C[Download to unique temporary path]
    B -->|Yes| D[Parse cached manifest]
    D -->|Parse fails| E[Evict cache entry]
    E --> C
    D -->|Parse succeeds| F[Compute and verify top hash]
    C --> G[Parse downloaded manifest]
    G --> F
    F -->|Missing hash or size| H[Raise contextual PackageException]
    F -->|Hash mismatch| I[Reject manifest]
    F -->|Verified| J[Promote temporary manifest to cache]
    J --> K[Return package]

Loading

_{Reviews (1): Last reviewed commit: "fix(quilt3): recover from a corrupt cach..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Python SDK package operations

---

Update 2026-08-26: retargeted to master, /code-review complete

/code-review on this branch returned 10 findings — 7 fixed in b29bc43db, 3 dismissed:

Fixed: the unverifiable-manifest error was a PackageException, which is not a QuiltException, so the CLI dumped a traceback instead of a one-line error (now QuiltException, via a private _ManifestUnverifiable subclass); a cached manifest that parses but can't be hashed was never evicted (now evicted, no retry — pinned by a test whose stubber fails loudly if a retry download happens); MemoryError is excluded from the corruption handler; eviction tolerates Windows sharing violations; the duplicated tmp-staging sequence became one closure; the path-equality cache-hit test became an explicit from_cache flag; tests reuse _filesystem_safe_encode.

Dismissed in writing:

1. Mismatch branch evicts without retrying, diverging from the parse branch — that's Verify browsed package manifest hashes #5184's deliberate, test-pinned semantics (test_remote_browse_rejects_manifest_top_hash_mismatch stubs no retry download). Generalizing it is @drernie's call, flagged here rather than changed.
2. Wrap the friendly error at _get_top_hash_parts instead of _browse — the source lacks the package/revision context the message needs, and it serves push/build paths where the browse framing would be wrong. The wrap interpolates the underlying message, so unusual causes (e.g. unsupported hash type) still surface their own text.
3. Structural manifest defects re-download on every browse since the cache never sticks — accepted: never serving a cache entry that fails to load outranks the bandwidth cost, and the error is loud each time.

Also noted for @drernie: the pre-existing mismatch error from #5184 raises PackageException, which the CLI's except QuiltException handler at main.py:523 doesn't catch — so quilt3 install of a tampered manifest prints a traceback. Same shape as finding 1 here, but in landed code; not changed in this PR.

Full run after everything: test_packages.py 232 passed, 1 xfailed; poe fmt --check and ruff clean.

[codecov]

Codecov Report

❌ Patch coverage is 95.23810% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.64%. Comparing base (213766a) to head (5d1c195).

Files with missing lines	Patch %	Lines
api/python/quilt3/packages.py	88.23%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5218      +/-   ##
==========================================
+ Coverage   54.55%   54.64%   +0.09%     
==========================================
  Files         871      871              
  Lines       36448    36527      +79     
  Branches     6438     6438              
==========================================
+ Hits        19883    19959      +76     
- Misses      14807    14810       +3     
  Partials     1758     1758              

Flag	Coverage Δ
api-python	93.84% <95.23%> (+0.01%)	⬆️
catalog	34.45% <ø> (ø)
lambda	85.37% <ø> (ø)
py-shared	98.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.