Skip to content

ci: set cache-mode on release workflows - #4370

Open
claude[bot] wants to merge 1 commit into
7.xfrom
ci/cache-mode
Open

claude[bot] wants to merge 1 commit into
7.xfrom
ci/cache-mode

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Requested by David Sanders · Slack thread

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable). — CI-only change; a one-line comment in each workflow explains the intent.
  • The changes have sufficient test coverage (if applicable). — not applicable; PR CI does not exercise the release workflows.
  • The testsuite passes successfully on my local machine (if applicable). — not applicable; no source changes.

Summarize your changes:

Before: Whether a release or docs-deploy run touched the GitHub Actions cache depended on per-step configuration. Publish (Forge 7) (push to main) and API Documentation (tag push v7.** / dispatch, which commits to the gh-pages site) both opt out step by step with package-manager-cache: false on setup-node, but nothing enforced that intent at the workflow level, so any future step or third-party action that restored a cache entry during a publish would silently be allowed.

After: Both workflows declare cache-mode: none at the top level, so every job in a release or docs-deploy run is denied cache access by the runner regardless of what individual steps or actions ask for. A denied cache restore logs a message and continues as a cache miss; a denied cache save logs and becomes a no-op, so neither workflow fails because of this setting. PR CI (ci.yml) is untouched and keeps caching as before.

GitHub's new workflow-level cache-mode key makes this a declarative, enforced setting rather than a convention (see changelog and workflow syntax reference), which closes the cache-poisoning avenue on the paths that publish to npm and to js.electronforge.io.

How: Adds cache-mode: none (with a one-line comment) directly after the top-level permissions: {} block in .github/workflows/release.yml and .github/workflows/gh-pages.yml. No other lines changed; the existing package-manager-cache: false lines are kept. Both files parse as YAML and zizmor 1.30.1 accepts the new key with no new findings. Note that PR CI does not exercise these workflows, so the first version-bump release (and the resulting v7.* tag push) after merge is the real test of this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MYe3WCXtgiqomNuVQdi4Yd


Generated by Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MYe3WCXtgiqomNuVQdi4Yd
@erickzhao
erickzhao marked this pull request as ready for review September 15, 2026 22:36
@erickzhao
erickzhao requested a review from a team as a code owner September 15, 2026 22:36
Comment on lines +10 to +11
# Release paths never touch the Actions cache; enforced regardless of per-step settings.
cache-mode: none

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🔴 Adding cache-mode: none at the workflow root can make GitHub treat the whole workflow file as invalid, so maintainers get zero npm publishes/GitHub releases after merge instead of the intended cache lockdown. GitHub's Actions parser enforces a fixed schema for the workflow root (name, on, permissions, env, defaults, concurrency, jobs) and rejects unknown top-level keys with "Unexpected value ''", failing validation before any job runs; cache-mode is not a documented workflow-level key. If that happens here, every push to main after a version bump fails validation and check-release-criteria/publish never execute, silently breaking releases. The same key is added at .github/workflows/gh-pages.yml:12, so a v7.* tag push would likewise never trigger docs deploy. …

Extended reasoning...

…Fix: verify cache-mode is an actual, currently-supported top-level workflow key before merging (it is not part of the schema as of this reviewer's knowledge); if unsupported, remove it and rely on the existing package-manager-cache:false/actions/cache exclusions instead.

GitHub validates each workflow YAML against a schema that only allows name, run-name, on, permissions, env, defaults, concurrency, jobs at the root; additional properties are rejected with an 'Unexpected value' validation error shown in the Actions tab, and none of the jobs run for that trigger. release.yml:10-11 and gh-pages.yml:11-12 both add cache-mode: none at that root level. If this key is not a real, currently-recognized field (the PR's zizmor 1.30.1 check only validates security lint rules, not GitHub's own workflow schema, so it would not catch this), the next push to main matching the release commit pattern fails workflow validation entirely: check-release-criteria and publish never run, so no npm publish and no GitHub release are created. Likewise the next v7.* tag push never runs gh-pages.yml's deploy job, so…

Verification: normal. The diff inserts cache-mode: none at the workflow root (release.yml:11 and gh-pages.yml:11), as a sibling of on:/permissions:/jobs:. GitHub Actions validates each workflow against a closed root schema (only name, run-name, on, permissions, env, defaults, concurrency, jobs); an unrecognized top-level property triggers a hard "Invalid workflow file … Unexpected value…

@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Confirming the reviewer's open question: cache-mode is a shipped, generally available workflow-syntax key as of 2026-09-10, valid at the workflow top level and at jobs.<job_id>.cache-mode, with values read, write, write-only, and none. Sources: changelog and workflow syntax reference. An unknown key would indeed fail workflow parsing, but this one is in the current schema.

As noted in the description, PR CI does not run release.yml, so the first tagged release after merge is the first real exercise of the setting. Per the docs, a cache operation not permitted under none "logs an informational message and continues. The job and workflow do not fail."


Generated by Claude Code

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants