diff --git a/CHANGELOG.md b/CHANGELOG.md index e86ffd9..fa8e72a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,18 @@ and could stop an installed copy from updating. ## [Unreleased] +### Added + +- **`rigging` node workflows cache the package manager's store.** The + generated `setup-node` step now carries the `cache` input derived from the + configured package manager (`npm`, `pnpm`, or `yarn` for yarn classic), so + CI runs restore the dependency store keyed on the lockfile instead of + installing cold every run (#45). bun gets no `cache` input because + setup-node does not support it, and yarn-berry gets none because setup-node + resolves the yarn cache directory during its own step — before rigging's + `corepack enable` runs — and would misdetect berry as the runner's Yarn + 1.22, caching a directory berry never reads. + ## [0.9.0] - 2026-07-23 ### Added diff --git a/plugins/rigging/rigging/plan.py b/plugins/rigging/rigging/plan.py index 4becf6f..99a7d90 100644 --- a/plugins/rigging/rigging/plan.py +++ b/plugins/rigging/rigging/plan.py @@ -125,15 +125,22 @@ def _build_job(stack_id: str, versions: tuple[str, ...], test_command: tuple[str, ...] | None = None, services: tuple[config.ResolvedService, ...] = ()) -> Job: spec = stacks.REGISTRY[stack_id] + setup_with = {spec.setup_with_key: "${{ matrix.%s }}" % spec.matrix_var} + if stack_id == "node": + # setup-node restores the manager's package store when told which + # manager keys it. Managers with cache_key None (bun, yarn-berry) get + # no `cache` input at all rather than a wrong one -- see the registry. + cache_key = stacks.NODE_PACKAGE_MANAGERS[manager_id].cache_key + if cache_key is not None: + setup_with["cache"] = cache_key setup_step = stacks.Step( uses=spec.setup_uses, uses_version=spec.setup_uses_version, - with_={spec.setup_with_key: "${{ matrix.%s }}" % spec.matrix_var}, + with_=setup_with, ) # The manager's own installer runs before setup-node, matching pnpm's - # documented order. Nothing here depends on it today (no dependency - # caching is configured), but the documented order is the one that stays - # correct if caching is ever added. + # documented order. The `cache` input depends on it: setup-node resolves + # the pnpm store path by running pnpm during its own step. manager_setup, manager_post_setup, manager_install = _manager_steps(stack_id, manager_id) test_argv = _resolve_test_argv(stack_id, manager_id, test_command) # An empty test argv would render `- run: ""` -- a silent no-op test step, diff --git a/plugins/rigging/rigging/stacks.py b/plugins/rigging/rigging/stacks.py index 22a01cc..bc163b8 100644 --- a/plugins/rigging/rigging/stacks.py +++ b/plugins/rigging/rigging/stacks.py @@ -74,6 +74,15 @@ class PackageManager: #: `setup_steps` rather than merged with an ordering flag, because "which #: side of setup-node" is the only question either field answers. post_setup_steps: tuple[Step, ...] = () + #: Value for setup-node's `cache` input, which restores this manager's + #: package store keyed on the lockfile. None means the input is omitted + #: entirely: setup-node's v7 source supports only npm/pnpm/yarn, so bun + #: has no value to give, and yarn-berry is excluded because setup-node + #: detects the yarn version by running `yarn --version` during its own + #: step -- before our `corepack enable` post-setup step has run -- so a + #: berry repo would be misdetected as the runner's Yarn 1.22 and a + #: classic cache directory saved that berry never reads. + cache_key: Optional[str] = None REGISTRY: dict[str, StackSpec] = { @@ -128,6 +137,7 @@ class PackageManager: lockfiles=("package-lock.json",), install=("npm", "ci"), test=("npm", "test"), + cache_key="npm", ), "pnpm": PackageManager( id="pnpm", @@ -139,6 +149,9 @@ class PackageManager: uses="pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271", uses_version="v6.0.9", ),), + # Cache resolution runs `pnpm store path` during setup-node, which + # only works because setup_steps put pnpm on the runner first. + cache_key="pnpm", ), # Yarn 1 and Yarn 2+ are two toolchains sharing one lockfile name, and # their install flags are mutually incompatible: --frozen-lockfile is an @@ -151,6 +164,7 @@ class PackageManager: lockfiles=("yarn.lock",), install=("yarn", "install", "--frozen-lockfile"), test=("yarn", "test"), + cache_key="yarn", ), "yarn-berry": PackageManager( id="yarn-berry", diff --git a/plugins/rigging/skills/init/SKILL.md b/plugins/rigging/skills/init/SKILL.md index 10e7322..f86a67c 100644 --- a/plugins/rigging/skills/init/SKILL.md +++ b/plugins/rigging/skills/init/SKILL.md @@ -82,6 +82,18 @@ non-null: (the flag berry installs with) is a Yarn 2+ flag that classic yarn errors on, so that step is what makes the runner capable of running berry's install line at all. + + The rendered `setup-node` step also carries a `cache` input for npm, pnpm, + and yarn classic, restoring the manager's package store keyed on the + lockfile. bun and yarn-berry get **no** `cache` input, and that is + deliberate, not missing support: setup-node cannot cache for bun at all, + and for berry it probes the yarn version during its own step — before the + `corepack enable` step above has run — so it would misdetect berry as the + runner's Yarn 1.22 and cache a directory berry never reads. If a user asks + why their bun or berry workflow "is missing caching", that is the answer. + Relatedly, the cache input assumes the lockfile is committed — but a + lockfile-less repo never gets this far, since detection refuses it and + `npm ci` could not run anyway. - **A reason** — this is a **refusal**, not a warning. Print it verbatim; it already names the files it found and what the user must do. Do not scaffold the node stack. If python is also detected, scaffold that alone and say diff --git a/plugins/rigging/tests/golden/node-mysql.yml b/plugins/rigging/tests/golden/node-mysql.yml index 6a443d7..7dcfbc1 100644 --- a/plugins/rigging/tests/golden/node-mysql.yml +++ b/plugins/rigging/tests/golden/node-mysql.yml @@ -27,5 +27,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/golden/node-pnpm.yml b/plugins/rigging/tests/golden/node-pnpm.yml index e59b29d..bf9f198 100644 --- a/plugins/rigging/tests/golden/node-pnpm.yml +++ b/plugins/rigging/tests/golden/node-pnpm.yml @@ -17,5 +17,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "pnpm" - run: "pnpm install --frozen-lockfile" - run: "pnpm test" diff --git a/plugins/rigging/tests/golden/node-postgres-database.yml b/plugins/rigging/tests/golden/node-postgres-database.yml index 1210d1b..d61f414 100644 --- a/plugins/rigging/tests/golden/node-postgres-database.yml +++ b/plugins/rigging/tests/golden/node-postgres-database.yml @@ -27,5 +27,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/golden/node-postgres.yml b/plugins/rigging/tests/golden/node-postgres.yml index 74021dc..b6a3b5b 100644 --- a/plugins/rigging/tests/golden/node-postgres.yml +++ b/plugins/rigging/tests/golden/node-postgres.yml @@ -27,5 +27,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/golden/node-redis.yml b/plugins/rigging/tests/golden/node-redis.yml index f6bae1b..8833a41 100644 --- a/plugins/rigging/tests/golden/node-redis.yml +++ b/plugins/rigging/tests/golden/node-redis.yml @@ -24,5 +24,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/golden/node-testcommand.yml b/plugins/rigging/tests/golden/node-testcommand.yml index fc305ba..0acdc6c 100644 --- a/plugins/rigging/tests/golden/node-testcommand.yml +++ b/plugins/rigging/tests/golden/node-testcommand.yml @@ -16,5 +16,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "turbo run test --concurrency=1" diff --git a/plugins/rigging/tests/golden/node-yarn1.yml b/plugins/rigging/tests/golden/node-yarn1.yml index ac8c905..9ab263a 100644 --- a/plugins/rigging/tests/golden/node-yarn1.yml +++ b/plugins/rigging/tests/golden/node-yarn1.yml @@ -16,5 +16,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "yarn" - run: "yarn install --frozen-lockfile" - run: "yarn test" diff --git a/plugins/rigging/tests/golden/node.yml b/plugins/rigging/tests/golden/node.yml index 69acbfd..d23faab 100644 --- a/plugins/rigging/tests/golden/node.yml +++ b/plugins/rigging/tests/golden/node.yml @@ -16,5 +16,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/golden/polyglot.yml b/plugins/rigging/tests/golden/polyglot.yml index 1f94980..b07fe2e 100644 --- a/plugins/rigging/tests/golden/polyglot.yml +++ b/plugins/rigging/tests/golden/polyglot.yml @@ -31,5 +31,6 @@ jobs: - uses: "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" # v7 with: node-version: "${{ matrix.node }}" + cache: "npm" - run: "npm ci" - run: "npm test" diff --git a/plugins/rigging/tests/test_plan.py b/plugins/rigging/tests/test_plan.py index 7ce9b3a..e195491 100644 --- a/plugins/rigging/tests/test_plan.py +++ b/plugins/rigging/tests/test_plan.py @@ -73,7 +73,7 @@ def test_node_job_wires_node_version_and_npm_steps(): stacks.Step( uses=stacks.REGISTRY["node"].setup_uses, uses_version=stacks.REGISTRY["node"].setup_uses_version, - with_={"node-version": "${{ matrix.node }}"}, + with_={"node-version": "${{ matrix.node }}", "cache": "npm"}, ), stacks.Step(run="npm ci"), stacks.Step(run="npm test"), @@ -133,3 +133,47 @@ def test_unset_manager_falls_back_to_the_default(): cfg = Config(name="ci", stacks={"node": StackConfig(versions=("20",))}) assert [s.run for s in build_plan(cfg).jobs[0].steps if s.run] == [ "npm ci", "npm test"] + + +def _setup_node_with(cfg): + """The setup-node step's with_ mapping for a single-job plan.""" + (job,) = build_plan(cfg).jobs + (setup,) = [s for s in job.steps + if s.uses == stacks.REGISTRY["node"].setup_uses] + return setup.with_ + + +@pytest.mark.parametrize("manager_id, cache_key", [ + ("npm", "npm"), + ("pnpm", "pnpm"), + ("yarn1", "yarn"), +]) +def test_setup_node_carries_the_managers_cache_input(manager_id, cache_key): + cfg = Config(name="ci", stacks={ + "node": StackConfig(versions=("20",), package_manager=manager_id)}) + assert _setup_node_with(cfg) == { + "node-version": "${{ matrix.node }}", "cache": cache_key} + + +@pytest.mark.parametrize("manager_id", ["yarn-berry", "bun"]) +def test_setup_node_omits_cache_when_the_manager_has_no_key(manager_id): + cfg = Config(name="ci", stacks={ + "node": StackConfig(versions=("20",), package_manager=manager_id)}) + assert _setup_node_with(cfg) == {"node-version": "${{ matrix.node }}"} + + +def test_cache_input_renders_after_node_version(): + """with_ renders in insertion order; node-version staying first keeps the + golden diff to a single added line.""" + cfg = Config(name="ci", stacks={"node": StackConfig(versions=("20",))}) + assert list(_setup_node_with(cfg)) == ["node-version", "cache"] + + +def test_python_setup_step_gains_no_cache_input(): + """The issue is node-only; setup-python's `cache: pip` is a separate + decision.""" + cfg = Config(name="ci", stacks={"python": StackConfig(versions=("3.12",))}) + (job,) = build_plan(cfg).jobs + (setup,) = [s for s in job.steps + if s.uses == stacks.REGISTRY["python"].setup_uses] + assert setup.with_ == {"python-version": "${{ matrix.python }}"} diff --git a/plugins/rigging/tests/test_stacks.py b/plugins/rigging/tests/test_stacks.py index f8c6a6c..8f6ee7c 100644 --- a/plugins/rigging/tests/test_stacks.py +++ b/plugins/rigging/tests/test_stacks.py @@ -303,3 +303,22 @@ def test_only_yarn_berry_needs_a_post_setup_step(): needing = {i for i, m in NODE_PACKAGE_MANAGERS.items() if m.post_setup_steps} assert needing == {"yarn-berry"} + + +def test_cache_keys_name_what_setup_node_can_actually_cache(): + """The cache_key is passed straight to setup-node's `cache` input, whose + v7 source supports exactly npm, pnpm and yarn -- so bun must stay None. + yarn-berry is None for a subtler reason: setup-node resolves the cache + directory by running `yarn --version` DURING its own step, and rigging + enables corepack after it, so berry would be detected as the runner's + Yarn 1.22 and a useless directory cached.""" + from rigging.stacks import NODE_PACKAGE_MANAGERS + + keys = {i: m.cache_key for i, m in NODE_PACKAGE_MANAGERS.items()} + assert keys == { + "npm": "npm", + "pnpm": "pnpm", + "yarn1": "yarn", + "yarn-berry": None, + "bun": None, + }