fix: close build coalescing race

[alecthomas]

Summary

• Re-check the local store after acquiring the per-key build lock.
• Restore a peer-committed artifact instead of compiling again when another process commits between the initial miss and lock acquisition.
• Update the architecture docs to show the post-lock re-check.

Why

kache already coalesces same-host in-flight Rust builds with a per-key lock, but there was a lookup-to-lock race: if project A committed an artifact after project B saw a miss but before B acquired the lock, B would acquire the now-free lock and compile the same artifact again. The new claim flow closes that window while preserving the existing wait-for-winner behavior under contention.

Verification

• cargo test --workspace
• cargo fmt --all -- --check
• git diff --check

A normal push was blocked by three unrelated pre-existing clippy warnings in remote_backend.rs, tui.rs, and wrapper.rs; the changed code was previously checked with those existing lints allowed.

[emmanuelm41]

Welcome, and nice PR — this is a real race and the fix is the right shape. Re-checking the store under the build lock is exactly how to close the window: once you hold the lock, no cooperating wrapper can commit before you compile, and if the previous owner committed just before releasing, your get() sees it and you restore instead of recompiling. The lock lifecycle is clean too — Committed holds the temporary guard and drops it exactly once, Acquired hands the guard to the wrapper, Contended never owns one — and the wrapper refactor that unifies the "committed → restore" path (shared by both the claim re-check and the post-contention wait) is a genuine simplification. Box<EntryMeta> is the right call for the enum size, and the diagnostics replay / LocalHit logging / clean_incremental_dir are all preserved.

One blocker before it lands: the Committed path doesn't guard against poisoned empty entries. An entry with files.is_empty() is a known bad state (from earlier bugs) that every other hit path defends against by evicting and recompiling — wrapper.rs:1184 ("Safety: skip entries with no cached files"), the cc path at :561, the daemon-hit path at :1881, and store.rs:366. But claim_build returns any committed entry as Committed(meta) without that check, and restore_from_cache on an empty files list iterates nothing, succeeds vacuously, and the wrapper returns Ok(0) — a "successful" build that produced no output, which then breaks the actual compile (cargo expects the .rlib/.o to exist). Before this change, that window would have re-acquired the released lock and compiled, so it's a new way to serve the poison.

The fix mirrors the existing guard, in claim_build while you still hold the lock:

match self.get(cache_key)? {
    Some(meta) if meta.files.is_empty() => {
        tracing::warn!("cache entry {} has no files, evicting before build", cache_key);
        self.remove_entry(cache_key)?;
        Ok(BuildClaim::Acquired(lock))   // compile instead of serving the poison
    }
    Some(meta) => Ok(BuildClaim::Committed(Box::new(meta))),
    None => Ok(BuildClaim::Acquired(lock)),
}

Two smaller, non-blocking notes:

• The regression test proves the re-check and the release, but it commits directly rather than modeling the real sequence (waiter misses → a peer holds the lock, commits, and releases → waiter claims). Driving it through a second Store handle's claim_build/drop makes it exercise the actual production path deterministically, no threads needed. And an empty_committed_entry test would lock in the fix above.
• Now that Err also covers a re-check get() failure after the lock was acquired, the "acquiring build lock ... failed" message is slightly off for that case — worth a word tweak, though the passthrough behavior is correct and safe.

Worth noting the scope this closes: it covers cooperating local wrapper writers. Daemon remote imports don't take the build lock, and stale-lock reclaim can still reap a lock older than an hour — both pre-existing, not this PR's job, just so the boundary is clear.

I read the code and traced the lock/restore paths; I didn't run a multi-process repro. Thanks again — this is a strong first contribution, just needs the empty-entry guard.

(AI-assisted review — a maintainer still owns the call.)