test: TEMPORARY - verify windows cache save (do not merge) - #22
Closed
BrandonLWhite wants to merge 6 commits into
Closed
test: TEMPORARY - verify windows cache save (do not merge)#22BrandonLWhite wants to merge 6 commits into
BrandonLWhite wants to merge 6 commits into
Conversation
Records the design for upgrading @actions/core to 3.x and migrating the codebase from CommonJS to ESM, which the upgrade forces. Covers the atomic three-package bump (core/cache/exec), the src/ and __tests__/ conversion, replacing @vercel/ncc with rollup, and the documentation constraints this inverts in AGENTS.md and .agents/INSTRUCTIONS.md. Also gitignores docs/superpowers/plans/, matching the convention that plans are ephemeral working docs while specs are preserved in git. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@actions/core 3.x, @actions/cache 6.x, and @actions/exec 3.x are ESM-only,
so require() of them throws at runtime. Migrate src/ and __tests__/ to
native ESM and bump all three together — @actions/cache@5 pins
@actions/core@^2, so bumping core alone resolves a second nested CommonJS
copy of core into the bundle.
ESM module namespace objects are frozen, so the test suite's
jest.spyOn(module, 'export') mocking no longer works. Tests now substitute
modules with jest.unstable_mockModule against fixtures in __fixtures__/.
Two fixtures are deliberately partial rather than pure stubs. Mocking a
module replaces it for every consumer, not just for the code under test:
__fixtures__/core.js re-exports the real @actions/core and overrides only
the asserted functions, because @actions/cache imports named exports from
it (setSecret among them) and a narrower mock makes main.test.js fail to
load. __fixtures__/fs-promises.js does the same to keep readFile real so
the TOML fixtures still load, mocking only symlink and stat.
Drops the jest.spyOn(main, 'run') self-spy from main.test.js, which ESM
makes impossible and which only asserted that run reached its end.
Coverage is unchanged: 9 tests, 100% statements/lines/functions, 92.85%
branches.
Verified the bundle loads and runs as ESM, not just that it builds:
env 'INPUT_INSTALL-CONFIG-FILE=__tests__/data/pyproject.empty.toml' \
node dist/index.js
prints "Nothing to install." and exits 0; a missing file emits ::error::
and exits 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Aligns the bundler with the upstream actions/javascript-action template
this repository derives from, and drops a largely dormant dependency. The
bundle shrinks from ~3.4MB to ~2.9MB.
Two deliberate deviations from the upstream config:
- @rollup/plugin-json is required. It is absent upstream, where the only
dependency is @actions/core. Here @actions/cache imports its own
package.json and the build fails outright without it.
- sourcemap is disabled. The generated map is ~16.5MB, and because dist/
is committed that would land in git history on every dependency bump
and bury the dist/ diff check-dist.yml reports.
rollup-plugin-license replaces ncc's --license flag, attributing 43
bundled packages (all MIT/ISC/Apache-2.0/BSD-3-Clause, matching the
allowed list in .licensed.yml). The bundle inlines the whole production
dependency tree, including the Azure Storage SDK @actions/cache@6 adds,
so those notices have to ship with it.
Removes dist/package.json, which only existed because ncc emits a
{"type":"module"} marker alongside an ESM bundle. dist/ is back to
exactly index.js and licenses.txt.
Verified the rollup bundle at runtime, not just at build time: the empty
pyproject fixture prints "Nothing to install." and exits 0, a missing file
emits ::error:: and exits 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moves Jest configuration out of package.json into jest.config.js and sets
transform: {}, which is correct for native ESM and means babel-jest is
never invoked.
Removes @babel/core, @babel/eslint-parser, @babel/preset-env, and
babel-preset-jest. There is no babel configuration anywhere in this
repository, so these have been inert since the original template
scaffold — applying the repository's own standing rule against carrying
unused dependencies.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes a silent bundling bug that broke @actions/cache's saveCache on
windows-latest only.
Several bundled dependencies probe for optional modules with require()
inside a try/catch: minimatch resolves `path` that way, and undici probes
node:http2 and node:crypto. @rollup/plugin-commonjs leaves requires inside
try/catch untouched by default, and in an ESM bundle `require` is
undefined, so the ReferenceError was swallowed by the empty catch and each
dependency silently took its fallback path. No error, no failed build.
For minimatch the fallback is `{ sep: '/' }`. minimatch only rewrites
backslashes to forward slashes when sep !== '/', so on Windows the file
paths @actions/glob feeds it were never normalized and stopped matching the
pattern. glob returned zero matches, and @actions/cache reported "Path
Validation Error: Path(s) specified in the action for caching do(es) not
exist" for C:\Program Files (x86)\pipx\venvs\poetry. Linux was unaffected
because sep is '/' there anyway.
This was a bundler regression from ncc, not from the @actions/cache major:
resolvePaths is byte-identical between cache 5.2.0 and 6.2.0, and minimatch
is 3.1.5 in both trees. ncc/webpack rewrites try/catch requires into its own
module registry, so the bundle it produced worked.
The bug was only reachable on a cache miss, which is why CI hid it — both
OS jobs normally restore from an existing key and skip saveCache. Confirmed
by salting the cache key on a throwaway branch to force a miss: windows
failed and ubuntu saved all three packages.
Adds __tests__/dist.test.js, which asserts the committed bundle contains no
executable bare require(). Verified it fails without this fix, catching all
six real offenders. Two documented pitfalls added to .agents/INSTRUCTIONS.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Not for merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BrandonLWhite
force-pushed
the
tmp/verify-windows-cache-save
branch
from
August 13, 2026 21:55
bd9fe62 to
a228a98
Compare
Member
Author
|
Verification complete. Confirmed the root cause was |
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
1 participant
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.
Throwaway PR. Salts the cache key to force a cache miss so the
saveCachepath actually runs on windows-latest, verifying whether@actions/cache6.x can save a cache to a path containing spaces and parentheses (C:\Program Files (x86)\pipx\venvs\poetry).Will be closed and the branch deleted once the answer is known.