Remove canonical-overlay cache mode

[frostebite]

Summary

Removes the canonical-overlay local cache mode entirely, per the decision in game-ci/roadmap#11 (workstream 1).

• localCacheMode: canonical-overlay now throws a clear error at config-read time instead of running, pointing users at move-directory or tar.
• Deletes canonical-cache-service.ts + its test, and all wiring in local-cache-service.ts and plugin-lifecycle.ts (including the now-unused canonicalCacheRoot, canonicalCacheClassifier, canonicalCacheVersionRetention, cacheMaterialize, cacheSentinelCanary inputs).
• tar, move-directory, and copy-directory local cache modes are unaffected.
• The existing rclone/S3 distributed-cache container hooks (docs/03-github-orchestrator/07-advanced-topics/05-hooks/05-built-in-hooks.mdx) are unaffected — this PR only touches the local, single-machine cache modes.

Net: -1287 lines, 5 files changed, no other behavior touched.

Why

Full audit and rationale live in game-ci/roadmap#11 (workstream 1) — summarizing the parts that motivated this specific PR:

What canonical-overlay did: kept one content-addressed copy of a cache folder (e.g. Library) per <cacheKey>/<folder>/<sha>/, computed via a structural SHA over relative paths+sizes. On publish, it copied/hardlinked the source into a staging directory, atomically renamed it into the canonical store, and flipped a latest junction/symlink. On restore, it hardlinked (or junction'd/copied, per a per-subtree classifier) from the canonical store into a per-job overlay directory.

Why it's being removed, not just deprioritized:

1. Silent perf cliff. Hardlink creation fails with EXDEV across filesystem volumes, and the fallback to a full copy was not logged at any level, not even debug — a user could be silently getting full-copy performance while believing they were getting zero-copy hardlinks, with no signal anywhere in the logs.
2. Doubled peak disk usage. Publish never removed or moved the source Library/ after copying it into the canonical store — the job's own copy was simply left behind, duplicated, on every publish.
3. Per-file syscall overhead at scale. Both publish (replicateTree) and the SHA computation (computeVersionSha) did a full recursive per-file walk with a syscall per file — for Library trees with 100k+ small files (common on large Unity projects, exactly the case this feature targeted), this overhead dominated even though bytes weren't being copied.
4. No concurrency safety. pruneOldVersions (sorts/removes canonical versions by mtime) had no locking against a concurrent materializeOverlay reading latest — two simultaneous publishers could race.
5. Meaningful added complexity (per-subtree classifier strategies, Windows junction ops shelled out via execSync('cmd /c ...'), a "prepared overlay" pre-fetch optimization) for a local-filesystem-only mechanism, when child-workspace-service.ts's much simpler fs.renameSync-based move approach already exists in this codebase and has none of the above problems.

Given all of that, the maintainer's call was to remove it outright rather than harden it in place, and let the simpler move-directory/tar/copy-directory modes (plus the existing rclone/S3 hooks for distributed caching) cover the space instead.

What's NOT in this PR (tracked separately in game-ci/roadmap#11)

• Switching the default LocalCacheMode from tar to move-directory — a separate, distinct behavior change from this removal, tracked as a follow-up.
• Adding an EXDEV → copy-directory automatic fallback for move-directory/child-workspace-service (today a cross-volume rename just throws) — follow-up.
• The CacheBackend interface design unifying local disk modes with the existing rclone/S3 hooks — a larger design task (RFC), not a mechanical removal.
• A cache warm / scheduled-hydration command — depends on the CacheBackend work above landing first.

Test plan

• yarn typecheck — clean, 0 errors
• yarn lint (oxlint) — 0 errors (390 pre-existing warnings, unrelated to this change)
• yarn test (vitest) — full suite: 60 files, 920 passed, 1 pre-existing skip, 0 failures
• Confirmed no other file in the repo imports canonical-cache-service after deletion
• Maintainer review of the deprecation error message / whether a softer migration path (e.g. one release cycle of a warning before throwing) is preferred over throwing immediately

Ref: game-ci/roadmap#11

Summary by CodeRabbit

• Changes
  • Removed support for the canonical-overlay local cache mode.
  • Local caching now supports only tar, copy-directory, and move-directory modes.
  • Removed configuration options for canonical cache setup and overlay behavior.
  • Added a clear error message when the removed mode is selected.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change removes canonical-overlay cache support from action inputs, environment mappings, local cache contracts, restore/save workflows, and lifecycle configuration. The lifecycle now rejects the removed mode and supports tar, copy-directory, and move-directory modes.

Changes

Canonical-overlay removal

Layer / File(s)	Summary
Update cache contracts and action configuration action.yml, src/model/orchestrator/services/cache/local-cache-service.ts	The action documents only supported cache modes and removes canonical cache environment mappings. Local cache options no longer define canonical-overlay settings.
Remove canonical-overlay cache workflows src/model/orchestrator/services/cache/local-cache-service.ts	Local cache restore and save paths no longer dispatch to canonical-overlay handling.
Reject removed mode at lifecycle configuration src/plugin-lifecycle.ts	localCacheMode throws an error for canonical-overlay and defaults to tar when no mode is configured.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: 🔵 Low · up to 7edee

The cache-mode removal is otherwise localized, but the deprecation error can be bypassed when local caching is disabled, leaving invalid configuration accepted silently; this is a bounded configuration-consistency risk requiring owner awareness or follow-up.

Possibly related PRs

• game-ci/orchestrator#11: Both changes modify local cache mode handling and LocalCacheService; this PR removes functionality introduced there.
• game-ci/orchestrator#20: Both changes modify local-cache restore/save behavior and lifecycle integration.
• game-ci/orchestrator#22: This PR removes canonical-overlay functionality introduced across the same cache and lifecycle areas.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the removal of the canonical-overlay cache mode, which is the main change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch remove-canonical-overlay-cache

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/model/orchestrator/services/cache/local-cache-service.ts (1)

223-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Match directory modes explicitly instead of "not tar".

Line 223 routes every non-tar value to restoreDirectoryCache. The caller in src/plugin-lifecycle.ts (lines 612-615) passes config.localCacheMode as any, so the compiler does not verify the value. An unknown or removed mode string silently selects directory restore rather than failing.

The lifecycle layer rejects canonical-overlay at config-read time, so this is not reachable today. An explicit allow-list keeps the dispatch safe if that guard changes. saveCacheFolder at line 331 has the same shape.

♻️ Proposed explicit mode matching

-      if (options.restoreMode && options.restoreMode !== 'tar') {
+      if (options.restoreMode === 'move-directory' || options.restoreMode === 'copy-directory') {
         return LocalCacheService.restoreDirectoryCache(projectPath, cachePath, folder, options);
       }

Apply the same change at line 331:

-      if (options.saveMode && options.saveMode !== 'tar') {
+      if (options.saveMode === 'move-directory' || options.saveMode === 'copy-directory') {

Then drop the as any casts in src/plugin-lifecycle.ts so LocalCacheMode is enforced at the call sites.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/model/orchestrator/services/cache/local-cache-service.ts` around lines
223 - 225, Update the mode dispatch in LocalCacheService restore and save flows
to call directory handling only for the explicitly supported directory mode,
rather than any value other than "tar"; preserve tar handling and reject or
otherwise avoid silently accepting unknown modes. Remove the as any casts at the
plugin-lifecycle call sites so LocalCacheMode is enforced by the type system.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/plugin-lifecycle.ts`:
- Around line 275-284: Move the canonical-overlay rejection from the cache
restore/save path into configuration construction, or explicitly evaluate the
lazy config.localCacheMode getter during lifecycle initialization. Ensure
canonical-overlay always throws during initialization, even when local caching
is disabled, while preserving valid mode handling.

---

Nitpick comments:
In `@src/model/orchestrator/services/cache/local-cache-service.ts`:
- Around line 223-225: Update the mode dispatch in LocalCacheService restore and
save flows to call directory handling only for the explicitly supported
directory mode, rather than any value other than "tar"; preserve tar handling
and reject or otherwise avoid silently accepting unknown modes. Remove the as
any casts at the plugin-lifecycle call sites so LocalCacheMode is enforced by
the type system.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ca6e254e-4246-4f51-bcfc-493396696f13

📥 Commits

Reviewing files that changed from the base of the PR and between b8e9cf8 and 7edee69.

📒 Files selected for processing (5)

• action.yml
• src/model/orchestrator/services/cache/canonical-cache-service.test.ts
• src/model/orchestrator/services/cache/canonical-cache-service.ts
• src/model/orchestrator/services/cache/local-cache-service.ts
• src/plugin-lifecycle.ts

💤 Files with no reviewable changes (2)

• src/model/orchestrator/services/cache/canonical-cache-service.test.ts
• src/model/orchestrator/services/cache/canonical-cache-service.ts

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map lifecycle declarations before inspecting configuration construction.
ast-grep outline src/plugin-lifecycle.ts --items all

# Find configuration initialization and every read of the lazy getter.
rg -n -C 5 '\blocalCacheMode\b|createPluginLifecycle|pluginLifecycle|const config|let config' src/plugin-lifecycle.ts

Repository: game-ci/orchestrator

Length of output: 3500

---

Validate canonical-overlay during configuration initialization.

config.localCacheMode is a lazy getter and is read only during cache restore and save. If local caching is disabled, canonical-overlay can bypass validation. Move this validation into configuration construction or force evaluation during lifecycle initialization.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugin-lifecycle.ts` around lines 275 - 284, Move the canonical-overlay
rejection from the cache restore/save path into configuration construction, or
explicitly evaluate the lazy config.localCacheMode getter during lifecycle
initialization. Ensure canonical-overlay always throws during initialization,
even when local caching is disabled, while preserving valid mode handling.