Fix cache path resolution to match prek's own behavior#102
Merged
Conversation
…E_HOME Query prek for its actual cache directory instead of hardcoding platform-specific paths in cache.ts. The fallback logic now mirrors prek's own resolution order: PREK_HOME (used as-is, with tilde expansion), then LOCALAPPDATA/prek on Windows, then XDG_CACHE_HOME/prek, then ~/.cache/prek. This fixes cache misses when prek uses a non-default cache location. Removes the duplicated getCachePaths helper in favor of a single getPrekCacheDir source of truth in prek.ts. Also replaces os.arch() with process.arch for consistency. The fallback exists because `prek cache dir` was added in v0.2.2 and `--no-log-file` in v0.2.3, so the CLI probe is unreliable on older versions. Consider dropping <v0.2.2 from the version manifest and removing the fallback once those versions are no longer in use.
04c0a7d to
c237d2e
Compare
There was a problem hiding this comment.
Pull request overview
Aligns this action’s cache save/restore behavior with prek’s own cache directory resolution by using prek cache dir --no-log-file as the source of truth (with a fallback for older prek versions).
Changes:
- Switch
restorePrekCache()to usegetPrekCacheDir()and remove the hardcodedgetCachePaths()implementation. - Add consolidated fallback resolution in
prek.ts(PREK_HOME / platform cache envs /~/.cache/prek) and exportgetPrekCacheDir(). - Update tests to cover the CLI probe and fallback paths; update bundled
dist/outputs.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/prek.ts |
Exports getPrekCacheDir() and adds getDefaultPrekCacheDir() fallback logic. |
src/cache.ts |
Uses getPrekCacheDir() for cache paths; removes getCachePaths(); uses process.arch in cache key. |
test/prek.test.ts |
Adds unit tests for CLI-probed cache dir and fallback behavior. |
test/cache.test.ts |
Updates restore cache tests to mock prek cache dir probe and assert resulting logs. |
dist/index.js |
Regenerated bundle reflecting the new cache-dir resolution flow. |
dist/post/index.js |
Regenerated post bundle reflecting the new cache-dir resolution flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
|
Thanks, good catch! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
restorePrekCacheused hardcoded platform paths (~/.cache/prekorLOCALAPPDATA/prek) to save and restore the prek hook cache. Aprek cache dirCLI probe already existed inprek.ts, but it was only used byshowVerboseLogs()to locateprek.log. This change makes the CLI probe the source of truth for cache save/restore as well, and fixes the fallback to match prek's actual resolution order.Problems
getCachePaths()incache.tshardcoded~/.cache/prek(Linux/macOS) andLOCALAPPDATA/prek(Windows). prek itself resolves its cache viaPREK_HOME, thenXDG_CACHE_HOME, then platform defaults. Any user withPREK_HOMEor a non-defaultXDG_CACHE_HOMEwould save/restore against the wrong directory.getPrekCacheDir()already queriedprek cache dir --no-log-file(the--no-log-fileflag prevents the probe itself from writing to prek's trace log). ButrestorePrekCachenever called it, using the hardcoded paths instead.getCachePathslived incache.ts,getPrekCacheDirlived inprek.ts, and they imported each other for their respective fallbacks.os.arch()vsprocess.archinconsistency. The cache key usedos.arch()while the rest of the codebase usesprocess.arch. They return the same value, butprocess.archis the standard convention in Node and avoids theosimport.Fixes
restorePrekCachenow callsgetPrekCacheDir(), which probesprek cache dir --no-log-filefirst.getCachePaths()entirely. The fallback logic is consolidated intogetDefaultPrekCacheDir()inprek.ts, mirroring prek's own resolution:PREK_HOME(used as-is with tilde expansion, no/preksuffix), thenLOCALAPPDATA/prekon Windows, thenXDG_CACHE_HOME/prek, then~/.cache/prek.os.arch()withprocess.archin the cache key.Future
The fallback exists because
prek cache dirwas added in v0.2.2 and--no-log-filein v0.2.3, so the CLI probe fails on older versions still in the version manifest (back to v0.0.23). Once pre-v0.2.2 versions are dropped from the manifest,getDefaultPrekCacheDircan be removed entirely.Test plan
getPrekCacheDir: CLI probe happy path,PREK_HOMEfallback,XDG_CACHE_HOME/LOCALAPPDATAfallback