Repository navigation
Harden Shared npm Workflows - #16
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request updates npm publishing and test coverage workflows. Publishing adds configurable npm versioning, OIDC trusted publishing, token bootstrap publishing, lifecycle scripts, job timeouts, and workspace package validation. Coverage testing adds configurable npm and Firebase CLI versions, Java 21, lockfile-based Firebase CLI validation, conditional emulator execution, emulator-only credential cleanup, and explicit coverage file handling. The README documents both workflows. Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant npmCLI
participant FirebaseCLI
participant npmRegistry
participant TestRunner
participant Codecov
GitHubActions->>npmCLI: Validate and install configured npm version
GitHubActions->>npmRegistry: Check package availability for token bootstrap
GitHubActions->>npmRegistry: Publish with OIDC or NPM_TOKEN
GitHubActions->>FirebaseCLI: Verify lockfile binary and optional version
GitHubActions->>TestRunner: Run regular or emulator tests
TestRunner->>Codecov: Generate coverage files
GitHubActions->>Codecov: Upload configured coverage files
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/npm-test-coverage.yml (1)
82-86: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftMake the Firebase CLI installation reproducible.
npm install --globalinstalls outside a lockfile. It pins the directfirebase-toolsversion, but transitive dependencies can change between runs. Use a lockfile-backed installation or another immutable tool artifact if this shared workflow requires a reproducible dependency tree.This addresses the
zizmoradhoc-packageswarning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/npm-test-coverage.yml around lines 82 - 86, Update the “Install Firebase CLI tools” workflow step to use a lockfile-backed or otherwise immutable Firebase CLI installation instead of global npm installation, while preserving the existing firebase-tools version input and conditional execution. Ensure the selected approach produces a reproducible transitive dependency tree and removes the adhoc package installation warning.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm-test-coverage.yml:
- Around line 109-111: Move the cleanup step that removes
google-application-credentials.json to immediately after the npm test step,
ensuring it runs with always() when the Firebase emulator setup is enabled. Keep
the existing RUNNER_TEMP path and removal command, and ensure cleanup occurs
before the codecov/codecov-action upload step.
- Around line 90-97: Scope GOOGLE_APPLICATION_CREDENTIALS_BASE64 to
emulator-enabled runs in the test step. Ensure it is empty or unavailable when
inputs.setup-firebase-emulator is false, so npm test and child processes cannot
receive the raw credential secret; preserve credential decoding and
GOOGLE_APPLICATION_CREDENTIALS setup for emulator runs.
---
Nitpick comments:
In @.github/workflows/npm-test-coverage.yml:
- Around line 82-86: Update the “Install Firebase CLI tools” workflow step to
use a lockfile-backed or otherwise immutable Firebase CLI installation instead
of global npm installation, while preserving the existing firebase-tools version
input and conditional execution. Ensure the selected approach produces a
reproducible transitive dependency tree and removes the adhoc package
installation warning.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f0db1708-95a8-4aba-8cff-135a807bdda6
📒 Files selected for processing (3)
.github/workflows/npm-publish.yml.github/workflows/npm-test-coverage.ymlREADME.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/npm-publish.yml (1)
69-69: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the timeout compatible with workspace publishing.
When
WORKSPACES=true, line 109 publishes all workspaces before the job timeout can cancel the run. NPM version/published name combinations cannot be reused, so a timeout after partial publication can leave an incomplete release that a retry cannot recover. Raise the job timeout for expected workspace publish time, or add resumable publishing and recovery logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/npm-publish.yml at line 69, Increase the workflow job’s timeout-minutes setting to allow the WORKSPACES=true publishing path at the npm publish step to complete before cancellation. Update the existing timeout configuration rather than adding unrelated recovery logic, while preserving the current publishing behavior.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm-publish.yml:
- Around line 20-25: Update the npmVersion workflow input and its installation
flow to accept only an approved exact semantic version, changing the default
from the mutable major-only “11” to an exact patch version. Validate the value
before invoking the npm installation command, reject tags, ranges, URLs, and Git
references, and add --ignore-scripts unless the workflow explicitly permits
installation scripts.
In `@README.md`:
- Line 510: Update the README documentation for npm-test-coverage.yml to state
that Firebase emulator support verifies firebase-tools version 15.26.0 by
default, and that callers enabling setup-firebase-emulator must lock the
matching version in package-lock.json.
---
Outside diff comments:
In @.github/workflows/npm-publish.yml:
- Line 69: Increase the workflow job’s timeout-minutes setting to allow the
WORKSPACES=true publishing path at the npm publish step to complete before
cancellation. Update the existing timeout configuration rather than adding
unrelated recovery logic, while preserving the current publishing behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 04282cc1-6fae-4ab3-9d67-b243e2009c45
📒 Files selected for processing (3)
.github/workflows/npm-publish.yml.github/workflows/npm-test-coverage.ymlREADME.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
491-491: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the optional npm version correctly.
At Line [491], the workflow does not always install npm 11.
npmVersionis optional, and an empty value uses the npm bundled with the selected Node.js release. State the optional exact version input and the bundled fallback.Suggested wording
-[`npm-publish.yml`](.github/workflows/npm-publish.yml) installs the requested npm 11 release, sets the package version, runs version lifecycle scripts, builds the package, and publishes it to npm with provenance. +[`npm-publish.yml`](.github/workflows/npm-publish.yml) optionally installs the exact npm version requested by the caller; when omitted, it uses the npm version bundled with the selected Node.js release. It sets the package version, runs version lifecycle scripts, builds the package, and publishes it to npm with provenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 491, Update the README description of npm-publish.yml to state that npmVersion accepts an optional exact npm 11 version and that leaving it empty uses the npm version bundled with the selected Node.js release.
🧹 Nitpick comments (1)
.github/workflows/npm-test-coverage.yml (1)
95-95: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftConstrain the globally installed npm artifact.
Line [95] fetches
npm@$NPM_VERSIONdirectly from the registry without a lockfile or integrity pin. An exact semver limits version selection, but it does not verify artifact contents. If less-trusted callers can setnpmVersion, use an approved-version allowlist or a vetted integrity manifest. Otherwise, document and enforce trusted callers for this input.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/npm-test-coverage.yml at line 95, Constrain the npm artifact installed by the workflow’s npm version input: validate NPM_VERSION against an approved-version allowlist or vetted integrity manifest before running the global npm install, and enforce that only trusted callers can provide it if no such validation exists. Keep the existing exact-version installation behavior after validation.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm-publish.yml:
- Around line 84-94: Update the npm publish workflow’s setup/preflight around
the “Set up npm” step to enforce npm version 9.5.0 or newer before running
publish, including when inputs.npmVersion is empty and the bundled npm is used.
Parse npm --version as a semantic version, reject older versions with a clear
error, and retain the existing exact-version validation for explicitly requested
npmVersion values.
In @.github/workflows/npm-test-coverage.yml:
- Around line 22-26: Update the npmVersion validation and description
consistently: either document that only stable MAJOR.MINOR.PATCH versions are
accepted and reject leading-zero components, or replace the current check with
complete semver validation so valid prerelease versions such as 12.0.2-beta.1
are accepted before installing npm@$NPM_VERSION.
---
Outside diff comments:
In `@README.md`:
- Line 491: Update the README description of npm-publish.yml to state that
npmVersion accepts an optional exact npm 11 version and that leaving it empty
uses the npm version bundled with the selected Node.js release.
---
Nitpick comments:
In @.github/workflows/npm-test-coverage.yml:
- Line 95: Constrain the npm artifact installed by the workflow’s npm version
input: validate NPM_VERSION against an approved-version allowlist or vetted
integrity manifest before running the global npm install, and enforce that only
trusted callers can provide it if no such validation exists. Keep the existing
exact-version installation behavior after validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 383c3c35-56b3-494d-a1b8-60da226cc4d3
📒 Files selected for processing (3)
.github/workflows/npm-publish.yml.github/workflows/npm-test-coverage.ymlREADME.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm-publish.yml:
- Around line 103-130: Update the version-validation regexes in the “Verify npm
supports trusted publishing” step to accept only stable X.Y.Z Node.js and npm
versions, removing the optional prerelease suffixes. Keep the existing numeric
minimum checks and error handling unchanged so versions such as v22.14.0-rc.1
and 11.5.1-rc.1 are rejected.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bd1eaca1-4842-45bd-9891-9a50f3c61cf5
📒 Files selected for processing (3)
.github/workflows/npm-publish.yml.github/workflows/npm-test-coverage.ymlREADME.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/npm-test-coverage.yml
### ♻️ Current situation & Problem The initial Grove consolidation preserves the source histories but also carries several independent build, lint, test, documentation, and publishing conventions. This follow-up establishes a consistent repository-wide baseline before the first 0.1.0 release. ### ⚙️ Release Notes - Standardize Node.js 24, npm 12, TypeScript 6, strict shared compiler settings, flat ESLint configuration, and root formatting. - Update dependencies to their latest compatible major versions, including Vite 8, Jest 30, and Vitest 4; remaining migrations are tracked with concrete blockers in [#2](#2). - Use current supported GitHub Action major tags throughout the repository. - Add SonarJS, zero-warning linting, stale-suppression detection, Knip dead-code and dependency analysis, dependency review, and CodeQL. - Migrate Firebase utilities and Cloud Messaging to Vitest while retaining Jest for the generated circular FHIR schema graph. - Validate package contents, exports, and declarations with tarball checks, Publint, and Are the Types Wrong. - Add package coverage thresholds, a 90% changed-code gate, REUSE validation, registry-signature verification, and documentation builds. - Normalize package output and synchronize internal workspace versions during release publishing. - Publish through npm Trusted Publishing by default, with an explicit token-only bootstrap for the first package publication. - Centralize contributor guidance in `AGENTS.md` and retain `CLAUDE.md` as a symlink. ### 📚 Documentation The root development and publishing instructions, package documentation, design-system site, badges, metadata, and release workflow have been updated for the unified repository. ### ✅ Testing - 703 tests pass across the design system and Firebase packages. - All package coverage gates pass; changed-code coverage is required at 90%. - Full, production-only, and documentation dependency audits report zero vulnerabilities at the low-severity threshold. - 1,297 installed packages have verified registry signatures and 305 have verified attestations. - Actionlint, YAML lint, Prettier, zero-warning ESLint, Knip, TypeScript, REUSE 3.3, Storybook, Docusaurus, package exports, declarations, and npm tarballs pass after a clean install. - An isolated 0.1.0 release simulation verifies synchronized workspace versions and internal dependencies. ### Review sequencing This PR currently calls the review branch from [SchmiedmayerLab/.github#16](SchmiedmayerLab/.github#16) so the new bootstrap input can be reviewed end to end. Merge and release that shared-workflow PR first, then replace the branch reference with its version tag before merging this PR. ### Code of Conduct & Contributing Guidelines By creating and submitting this pull request, you agree to follow our [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md).
♻️ Current situation & Problem
The shared npm workflows needed tighter credential handling, explicit provenance permissions, bounded execution, and reproducible tool controls.
This is a focused follow-up to adopting the workflows in
SchmiedmayerLab/grove-ts; no issue was required for the contained maintenance change.⚙️ Release Notes
bootstrapWithTokenmode that creates unpublished packages and refuses packages already present on npm.NPM_TOKENoptional and expose it only to bootstrap steps; normal publishing has no token fallback.firebase-toolsand derive the emulator cache from the lockfile.📚 Documentation
The workflow reference documents normal token-free publishing, one-time bootstrap usage, trusted publisher setup, optional npm controls, explicit coverage reports, and lockfile-backed Firebase tooling.
✅ Testing
actionlintyamllint -c .yamllint.yml .github/workflows/npm-publish.yml .github/workflows/npm-test-coverage.ymlreuse --no-multiprocessing lintgit diff --checkAfter this PR is released,
SchmiedmayerLab/grove-ts#1will replace its temporary review-branch reference with the new version tag before merging.Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: