Skip to content

Add explicit cache input to enable/disable caching#111

Merged
j178 merged 4 commits into
j178:mainfrom
shaanmajid:feat/cache-input
Mar 18, 2026
Merged

Add explicit cache input to enable/disable caching#111
j178 merged 4 commits into
j178:mainfrom
shaanmajid:feat/cache-input

Conversation

@shaanmajid

Copy link
Copy Markdown
Collaborator

Should seldom be manually set, but a worthwhile failsafe for debugging or working around broken environments.

@shaanmajid shaanmajid marked this pull request as ready for review March 18, 2026 06:39
@shaanmajid shaanmajid added the enhancement New feature or request label Mar 18, 2026
@j178

j178 commented Mar 18, 2026

Copy link
Copy Markdown
Owner

Should we avoid save-cache if cache: false?

@shaanmajid

shaanmajid commented Mar 18, 2026

Copy link
Copy Markdown
Collaborator Author

Should we avoid save-cache if cache: false?

It already does this, albeit a bit indirectly.

When cache: false, we skip calling savePrekCache(), which is the only function which sets the CACHE_KEY_STATE or CACHE_PATHS_STATE state vars.

Although we still call restorePrekCache() in the post job unconditionally, it short-circuits when either CACHE_KEY_STATE or CACHE_PATHS_STATE are falsey. So, in practice, we never save the cache when the cache input is false.

I can make it more explicit if you prefer (by re-fetching the input in post.ts, but this seemed to be pretty consistent with how some other actions do it.

@j178

j178 commented Mar 18, 2026

Copy link
Copy Markdown
Owner

Ah, got it. I think it’d be better if it were more explicit, maybe add a few comments to clarify?

@shaanmajid

Copy link
Copy Markdown
Collaborator Author

Done, also updated log messages

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new cache action input to explicitly enable/disable prek environment caching, primarily as a troubleshooting/failsafe switch while keeping caching enabled by default.

Changes:

  • Introduces cache boolean input (default true) in action.yaml, parsing it via getInputs(), and wiring it into main execution to skip cache restore when disabled.
  • Updates cache post-step behavior/logging to cleanly no-op when cache state was never initialized.
  • Adds/updates unit tests and documentation to cover the new input and behavior.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/main.test.ts Adds coverage ensuring cache restore is skipped and cache-hit=false is emitted when caching is disabled.
test/inputs.test.ts Adds coverage for default cache behavior and opt-out parsing.
test/cache.test.ts Updates expectation for revised “no cache state” logging when save is skipped.
src/types.ts Extends Inputs type with cache: boolean.
src/inputs.ts Parses the new cache input via core.getBooleanInput('cache').
src/main.ts Gates restorePrekCache() behind inputs.cache, and sets cache-hit appropriately when disabled.
src/cache.ts Clarifies/adjusts post-step “no state” log message for cases where restore never ran (including cache disabled).
src/post.ts Adds comment explaining post-step always runs and save is conditional on state.
action.yaml Defines the new cache input with default "true".
README.md Documents the new cache input in the inputs table.
dist/index.cjs Updates bundled output for new input + conditional restore behavior.
dist/post/index.cjs Updates bundled output for revised cache-save no-op logging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@j178 j178 merged commit b42be1f into j178:main Mar 18, 2026
13 checks passed
@j178

j178 commented Mar 18, 2026

Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants