Files
multica/server
Bohan Jiang 20d2dd8cef MUL-5620: fix(daemon): close the repo-eviction liveness window and stop GC walking internal caches (#6299)
* fix(daemon): close the repo-eviction liveness window and stop GC walking internal caches

Two review follow-ups from #6297.

1. The live-repo set was snapshotted once at the start of the .repos walk,
   then consulted per repo after running git and filesystem work on each
   one in turn. A workspace re-attaching a repo inside that window could
   see the cache evicted anyway. Replace the snapshot with a per-path
   query and ask twice: once as a cheap early-out so an attached repo
   never pays for the git work, and again immediately before RemoveAll.
   Re-reading in-memory state costs one mutex and no network, and shrinks
   the window from the whole walk to a few adjacent statements.

   This narrows the race rather than eliminating it — attachment updates
   workspaceState without taking the repo lock, so a sufficiently unlucky
   interleaving is still possible. It stays benign: a freshly attached
   repo has no last-used stamp and takes the backfill-and-skip path, and
   a wrong eviction costs one re-clone via ensureRepoReady.

2. runGC skipped only .repos, so it walked .skill-cache as if it were a
   workspace. Its "v1" directory then looked like a task dir with no
   .gc_meta.json and the orphan path deleted the entire bundle cache once
   its mtime went GCOrphanTTL without a new bundle — a few hundred KB
   reclaimed in exchange for a full re-download. Skip every dot-directory,
   matching what ScanDiskUsage already does; workspace directories are
   always UUIDs, so a dot-prefixed entry is one of our own caches.

   The skill cache has no lifecycle of its own, but it is measured in
   hundreds of KB, so leaving it unmanaged is clearly better than deleting
   it wholesale on an unrelated TTL. Giving it a real lifecycle is
   separate work if it ever grows.

Steve's third observation — repoCacheSize measuring any second-level
directory while eviction only handles isBareRepo ones — is left as is on
purpose: measuring more widely than we delete is the right asymmetry for
a visibility feature, so a corrupted leftover stays visible instead of
silently uncounted.

Co-authored-by: multica-agent <github@multica.ai>

* fix(daemon): measure repo size before the final liveness check, not after

Review follow-up: the second liveness check was not actually adjacent to
the delete. dirSize walks every file in the bare repo and ran between the
check and RemoveAll, so on a multi-GiB cache — the size that motivated
this work in the first place — a workspace re-attaching during that walk
would still lose its cache.

Measure first, then check, then delete. That leaves only the window
between two adjacent statements, which is what the comment claimed all
along, and it keeps the slow filesystem walk outside the section the
check is meant to protect.

No behaviour change beyond the ordering: bytes_reclaimed still reports
the size measured just before deletion, which
TestEvictRepoCache_RemovesIdleDetachedRepo already asserts is non-zero.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-08-03 15:34:33 +08:00
..