fix(frontend-pr-analysis): detect vitest version for coverage reporter flag#219
Merged
fix(frontend-pr-analysis): detect vitest version for coverage reporter flag#219
Conversation
…r flag compatibility Vitest 3 removed --coverageReporters CLI flag. Detect the installed Vitest major version at runtime and use --coverage.reporter (dot notation) for v3+, keeping --coverageReporters for Jest and Vitest 2. Closes #215
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughDetects Vitest major version at runtime to select compatible coverage CLI flags; pins several GitHub Actions to specific commit SHAs; tightens Git quoting and ref handling for PR diffs; and consolidates writes to GITHUB_OUTPUT and GITHUB_STEP_SUMMARY into grouped append blocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
lerian-studio
added
size/XS
🔍 Lint Analysis
|
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Around line 369-380: The workflow currently sets Vitest-specific
COVERAGE_FLAGS based only on VITEST_MAJOR and then invokes the package manager
test command (see VITEST_MAJOR, COVERAGE_FLAGS and the case that runs
yarn|pnpm|npm test), which breaks when the repository's "test" script uses Jest
or another runner; change the logic to first detect the actual test runner used
by the repository (e.g., inspect the test script or installed devDependencies to
distinguish Vitest vs Jest) and only assign Vitest reporter flags to
COVERAGE_FLAGS when Vitest is actually used; for Jest or unknown runners do not
inject Vitest reporter flags (use plain --coverage or no extra flags), and
remove the invalid fallback flag name (--coverageReporters) so that the case
block that runs yarn test / pnpm test / npm test passes only flags appropriate
to the detected runner.
🪄 Autofix (Beta)
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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b2e34d77-fc83-40f8-bc59-773132bc5cf6
📒 Files selected for processing (1)
.github/workflows/frontend-pr-analysis.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/frontend-pr-analysis.yml (1)
369-380:⚠️ Potential issue | 🟠 MajorGate coverage reporter args by actual runner, not by resolved Vitest package.
Line 369 detects only whether
vitestis resolvable, but Lines 371-380 always inject Vitest-style/Jest-style reporter flags into a generictestscript. In monorepos with hoisted Vitest and a Jest test script, this can pass--coverage.reporter=*to Jest and fail CI. That is a breaking behavior for external callers.Suggested fix
+ TEST_SCRIPT=$(node -e "try{const p=require('./package.json');process.stdout.write((p.scripts&&p.scripts.test)||'')}catch{process.stdout.write('')}" 2>/dev/null) VITEST_MAJOR=$(node -e "try{console.log(require('vitest/package.json').version.split('.')[0])}catch{console.log('0')}" 2>/dev/null) - if [[ "$VITEST_MAJOR" -ge 3 ]]; then - COVERAGE_FLAGS=(--coverage.reporter=text --coverage.reporter=json-summary --coverage.reporter=lcov) - else - COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) - fi + COVERAGE_FLAGS=() + if [[ "$TEST_SCRIPT" == *vitest* ]]; then + if [[ "$VITEST_MAJOR" -ge 3 ]]; then + COVERAGE_FLAGS=(--coverage.reporter=text --coverage.reporter=json-summary --coverage.reporter=lcov) + else + COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) + fi + elif [[ "$TEST_SCRIPT" == *jest* ]]; then + COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) + fiUsing official docs, confirm runner-specific support for coverage reporter CLI flags: 1) Vitest v2/v3: `--coverage.reporter` vs `--coverageReporters` 2) Jest (current): `--coverage.reporter` vs `--coverageReporters`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/frontend-pr-analysis.yml around lines 369 - 380, The current logic sets COVERAGE_FLAGS based solely on whether Vitest is resolvable (VITEST_MAJOR) causing Vitest-style flags to be passed to whatever the repo's "test" script runs; instead determine the actual test runner by inspecting the package's test script (package.json "scripts.test") or by checking for an explicit "jest" or "vitest" invocation in that script, then set COVERAGE_FLAGS accordingly (use Vitest flags (--coverage.reporter / --coverageReporters) only if the script invokes "vitest" and use Jest flags (the appropriate Jest coverage CLI form) only if the script invokes "jest"); modify the logic around VITEST_MAJOR and COVERAGE_FLAGS and branch before the case for inputs.package_manager so the package_manager switch simply runs the test command with the already-chosen, runner-appropriate COVERAGE_FLAGS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Around line 369-380: The current logic sets COVERAGE_FLAGS based solely on
whether Vitest is resolvable (VITEST_MAJOR) causing Vitest-style flags to be
passed to whatever the repo's "test" script runs; instead determine the actual
test runner by inspecting the package's test script (package.json
"scripts.test") or by checking for an explicit "jest" or "vitest" invocation in
that script, then set COVERAGE_FLAGS accordingly (use Vitest flags
(--coverage.reporter / --coverageReporters) only if the script invokes "vitest"
and use Jest flags (the appropriate Jest coverage CLI form) only if the script
invokes "jest"); modify the logic around VITEST_MAJOR and COVERAGE_FLAGS and
branch before the case for inputs.package_manager so the package_manager switch
simply runs the test command with the already-chosen, runner-appropriate
COVERAGE_FLAGS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb2ac9c5-3149-4442-9bf1-7ddad587d7a1
📒 Files selected for processing (1)
.github/workflows/frontend-pr-analysis.yml
- Pin all external actions by SHA (checkout, setup-node, upload-artifact, download-artifact, github-script) - Quote $GITHUB_OUTPUT to satisfy SC2086 - Replace $? check with direct exit code test to satisfy SC2181 - Quote $BASE_SHA, $HEAD_SHA, $PREV_COMMIT variables
lerian-studio
added
size/S
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Group GITHUB_OUTPUT writes with { } >> to satisfy SC2129
- Quote $GITHUB_STEP_SUMMARY to satisfy SC2086
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/frontend-pr-analysis.yml (1)
366-379:⚠️ Potential issue | 🟠 MajorMake coverage flags runner-aware; current Vitest detection can apply wrong CLI flags.
Line 368 keys behavior off installed Vitest, not the actual
testrunner command. In mixed setups, this can pass Vitest-only flags to Jest/other runners (or vice versa). Keep Vitest flags only when the test script is Vitest; otherwise avoid injecting reporter flags unless runner support is explicit.Suggested fix
- # Vitest 3+ removed --coverageReporters CLI flag (use --coverage.reporter instead) - # Jest and Vitest 2 still support --coverageReporters - VITEST_MAJOR=$(node -e "try{console.log(require('vitest/package.json').version.split('.')[0])}catch{console.log('0')}" 2>/dev/null) - - if [[ "$VITEST_MAJOR" -ge 3 ]]; then - COVERAGE_FLAGS=(--coverage.reporter=text --coverage.reporter=json-summary --coverage.reporter=lcov) - else - COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) - fi + TEST_SCRIPT=$(node -e "try{const s=require('./package.json').scripts?.test||'';process.stdout.write(s)}catch{process.stdout.write('')}" 2>/dev/null) + VITEST_MAJOR=$(node -e "try{console.log(require('vitest/package.json').version.split('.')[0])}catch{console.log('0')}" 2>/dev/null) + COVERAGE_FLAGS=() + + if [[ "$TEST_SCRIPT" == *vitest* ]] && [[ "$VITEST_MAJOR" -ge 1 ]]; then + COVERAGE_FLAGS=(--coverage.reporter=text --coverage.reporter=json-summary --coverage.reporter=lcov) + elif [[ "$TEST_SCRIPT" == *jest* ]]; then + COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) + fiFor current official docs: (1) In Vitest v2 and v3, which CLI flag is supported for coverage reporters: `--coverage.reporter` or `--coverageReporters`? (2) In Jest 30, is `coverageReporters` supported via CLI args (camelCase config passthrough), and what syntax is documented?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/frontend-pr-analysis.yml around lines 366 - 379, The script currently sets COVERAGE_FLAGS based on the presence/version of installed Vitest (VITEST_MAJOR) which can misapply Vitest-only CLI flags to other test runners; update the logic to detect the actual test runner invoked by inspecting the test script command in package.json (e.g., read the "scripts.test" value) and only select Vitest-style reporters (--coverage.reporter) when that script contains "vitest", otherwise select runner-appropriate flags (or leave REPORTERS empty) for Jest/others; adjust the variables (e.g., VITEST_MAJOR, COVERAGE_FLAGS) and the case block that runs yarn/pnpm/npm test so it forwards the correct flags only when the detected test runner matches (and preserve correct argument quoting/precedence for npm -- vs yarn/pnpm).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Line 116: The fallback git diff invocation that interpolates `${{
github.base_ref }}` directly is vulnerable to ref injection and parsing errors;
map the base ref through an env variable (e.g., BASE_REF) in the workflow `env:`
section, validate/sanitize it (allow only safe characters like [A-Za-z0-9._/-]
or reject otherwise) and then use the quoted variable in the FILES assignment
(replace occurrences of `origin/${{ github.base_ref }}` with
`origin/"$BASE_REF"`), ensuring the variable is always quoted when passed to
`git diff --name-only` so `$BASE_REF` cannot break the shell or git arguments.
- Around line 422-431: The repeated echo appends to "$GITHUB_OUTPUT" (using
COVERAGE, STATEMENTS, BRANCHES, FUNCTIONS) trigger shellcheck SC2129; replace
the multiple lines that each do echo "... >> \"$GITHUB_OUTPUT\"" with a single
grouped redirection or here-doc so all four outputs are written in one >>
operation (e.g., group the four echo/printf lines inside { ...; } >>
"$GITHUB_OUTPUT" or use a single cat <<EOF >> "$GITHUB_OUTPUT" block) and apply
the same change to the else branch that writes zeros.
---
Duplicate comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Around line 366-379: The script currently sets COVERAGE_FLAGS based on the
presence/version of installed Vitest (VITEST_MAJOR) which can misapply
Vitest-only CLI flags to other test runners; update the logic to detect the
actual test runner invoked by inspecting the test script command in package.json
(e.g., read the "scripts.test" value) and only select Vitest-style reporters
(--coverage.reporter) when that script contains "vitest", otherwise select
runner-appropriate flags (or leave REPORTERS empty) for Jest/others; adjust the
variables (e.g., VITEST_MAJOR, COVERAGE_FLAGS) and the case block that runs
yarn/pnpm/npm test so it forwards the correct flags only when the detected test
runner matches (and preserve correct argument quoting/precedence for npm -- vs
yarn/pnpm).
🪄 Autofix (Beta)
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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91895350-73c8-4e32-9185-1de6b0147244
📒 Files selected for processing (1)
.github/workflows/frontend-pr-analysis.yml
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/frontend-pr-analysis.yml (2)
116-116:⚠️ Potential issue | 🟠 MajorHarden fallback diff ref handling at Line 116.
origin/${{ github.base_ref }}...HEADis still interpolated directly in shell. Map it throughenv, validate it as a branch name, and quote it before use.Suggested fix
- name: Get changed files id: changed shell: bash + env: + BASE_REF: ${{ github.base_ref }} run: | if [[ "${{ github.event_name }}" == "pull_request" ]]; then BASE_SHA="${{ github.event.pull_request.base.sha }}" HEAD_SHA="${{ github.event.pull_request.head.sha }}" git fetch origin "$BASE_SHA" --depth=1 2>/dev/null || true - FILES=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA" 2>/dev/null || git diff --name-only origin/${{ github.base_ref }}...HEAD) + if ! git check-ref-format --branch "$BASE_REF" >/dev/null 2>&1; then + echo "::error::Invalid base ref: $BASE_REF" + exit 1 + fi + FILES=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA" 2>/dev/null || git diff --name-only "origin/${BASE_REF}...HEAD")As per coding guidelines, "Validate branch names and label names before using in shell commands; always quote variables and map through
env:."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/frontend-pr-analysis.yml at line 116, The FILES assignment uses an unquoted interpolated ref origin/${{ github.base_ref }}...HEAD directly in the shell; change it to read the base ref through an environment variable (e.g., export BASE_REF="${{ github.base_ref }}" via env:), validate/sanitize BASE_REF against a safe branch-name regex (allow only [A-Za-z0-9._/-] and reject/exit if invalid or empty), assign a fallback variable like FALLBACK_REF="origin/$BASE_REF...HEAD" and then use the quoted variable in the git diff command (git diff --name-only "$BASE_SHA" "$HEAD_SHA" 2>/dev/null || git diff --name-only "$FALLBACK_REF"...HEAD) so the ref is validated and always quoted when used; update references to BASE_SHA, HEAD_SHA, BASE_REF, and FILES accordingly.
366-379:⚠️ Potential issue | 🟠 MajorDo not infer runner solely from installed Vitest version (Lines 366-379).
Current logic can select Vitest v3 flags even when the
testscript actually runs Jest/another runner in mixed-tool repos. Detect the test runner first (frompackage.jsonscript), then apply Vitest-specific flags only when Vitest is the executed runner.Suggested fix
- VITEST_MAJOR=$(node -e "try{console.log(require('vitest/package.json').version.split('.')[0])}catch{console.log('0')}" 2>/dev/null) + TEST_SCRIPT=$(node -e "try{const p=require('./package.json');process.stdout.write((p.scripts&&p.scripts.test)||'')}catch{process.stdout.write('')}" 2>/dev/null) + VITEST_MAJOR=$(node -e "try{console.log(require('vitest/package.json').version.split('.')[0])}catch{console.log('0')}" 2>/dev/null) - if [[ "$VITEST_MAJOR" -ge 3 ]]; then + if [[ "$TEST_SCRIPT" == *vitest* ]] && [[ "$VITEST_MAJOR" -ge 3 ]]; then COVERAGE_FLAGS=(--coverage.reporter=text --coverage.reporter=json-summary --coverage.reporter=lcov) else COVERAGE_FLAGS=(--coverageReporters=text --coverageReporters=json-summary --coverageReporters=lcov) fiAs of the latest docs, which CLI flags are valid for setting coverage reporters in: 1) Vitest v2 2) Vitest v3+ 3) Jest Please include official docs links for each.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/frontend-pr-analysis.yml around lines 366 - 379, The workflow currently infers Vitest flags from VITEST_MAJOR alone; instead, read the repository's test script and detect the actual test runner before setting COVERAGE_FLAGS: inspect package.json's "scripts"."test" (or the npm_package_scripts_test env) for tokens like "vitest" (and fallback to jest) and only assign Vitest-specific flags when the test script actually invokes vitest; otherwise keep Jest/pnpm/yarn-compatible flags. Update the logic around VITEST_MAJOR and COVERAGE_FLAGS to first set a RUNNER variable (e.g., detect "vitest" vs "jest") using the package.json test script, then conditionally populate COVERAGE_FLAGS based on RUNNER and VITEST_MAJOR, and leave the case for inputs.package_manager and the test invocation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/frontend-pr-analysis.yml:
- Line 116: The FILES assignment uses an unquoted interpolated ref origin/${{
github.base_ref }}...HEAD directly in the shell; change it to read the base ref
through an environment variable (e.g., export BASE_REF="${{ github.base_ref }}"
via env:), validate/sanitize BASE_REF against a safe branch-name regex (allow
only [A-Za-z0-9._/-] and reject/exit if invalid or empty), assign a fallback
variable like FALLBACK_REF="origin/$BASE_REF...HEAD" and then use the quoted
variable in the git diff command (git diff --name-only "$BASE_SHA" "$HEAD_SHA"
2>/dev/null || git diff --name-only "$FALLBACK_REF"...HEAD) so the ref is
validated and always quoted when used; update references to BASE_SHA, HEAD_SHA,
BASE_REF, and FILES accordingly.
- Around line 366-379: The workflow currently infers Vitest flags from
VITEST_MAJOR alone; instead, read the repository's test script and detect the
actual test runner before setting COVERAGE_FLAGS: inspect package.json's
"scripts"."test" (or the npm_package_scripts_test env) for tokens like "vitest"
(and fallback to jest) and only assign Vitest-specific flags when the test
script actually invokes vitest; otherwise keep Jest/pnpm/yarn-compatible flags.
Update the logic around VITEST_MAJOR and COVERAGE_FLAGS to first set a RUNNER
variable (e.g., detect "vitest" vs "jest") using the package.json test script,
then conditionally populate COVERAGE_FLAGS based on RUNNER and VITEST_MAJOR, and
leave the case for inputs.package_manager and the test invocation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b4d4aaf-a598-432a-aab2-2183ac6ad114
📒 Files selected for processing (1)
.github/workflows/frontend-pr-analysis.yml
…ions to env vars
Move ${{ }} expressions out of run: blocks into env: mappings to prevent
potential code injection via user-controlled inputs (github.base_ref,
matrix.app.name, coverage outputs, inputs.coverage_threshold).
Resolves all 7 CodeQL code-injection/medium findings.
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
3 participants
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.
GitHub Actions Shared Workflows
Description
Vitest 3 removed the
--coverageReportersCLI flag, causing thefrontend-pr-analysisworkflow to fail withUnknown option --coverageReportersfor any repository that upgrades to Vitest 3+.This PR detects the installed Vitest major version at runtime and selects the appropriate CLI syntax:
--coverage.reporter=X(dot notation)--coverageReporters=X(original syntax)If Vitest is not installed (e.g., project uses Jest), the detection safely falls back to the original flags.
Type of Change
fix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)Breaking Changes
None.
Testing
@developor the beta tagCaller repo / workflow run: https://github.com/LerianStudio/lerian-map/pull/144
Related Issues
Closes #215
Summary by CodeRabbit
Chores
Tests