feat(action): add @node-minify/action GitHub Action package for Marketplace#2760
feat(action): add @node-minify/action GitHub Action package for Marketplace#2760
Conversation
🦋 Changeset detectedLatest commit: fad512f The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds a packaged Node‑20 GitHub Action for node-minify, deprecates the legacy composite action, refactors gzip utilities and compressor resolution, adds "oxc" to benchmarks, and introduces new docs, tests, and CI workflows for building, releasing, and testing the action. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant GH as GitHub Actions
participant Action as node-minify Action
participant Parser as parseInputs()
participant Resolver as resolveCompressor (utils)
participant Minifier as runMinification/runBenchmark
participant Checks as checkThresholds()
participant Reporter as reporters (summary/comment/annotations)
User->>GH: trigger workflow with inputs
GH->>Action: execute action entrypoint (packages/action/dist)
Action->>Parser: parse inputs & validate compressor
Parser->>Resolver: validate/resolve compressor metadata
Resolver-->>Action: compressor info
Action->>Minifier: runMinification (and optional runBenchmark)
Minifier->>Resolver: resolveCompressor for implementation
Minifier-->>Action: MinifyResult / BenchmarkResult
Action->>Checks: checkThresholds()
alt Threshold violated
Action->>GH: fail action
else
Action->>Reporter: set outputs
Reporter->>GH: publish outputs and summary
opt reportPRComment
Action->>Reporter: postPRComment()
Reporter->>GH: create/update PR comment
end
opt reportAnnotations
Action->>Reporter: addAnnotations()
Reporter->>GH: emit annotations
end
end
GH-->>User: final workflow status
sequenceDiagram
participant Workflow as GitHub Workflow
participant Build as build-action job
participant Artifact as Artifact Storage
participant Test as test jobs
Workflow->>Build: run build-action (checkout, setup Bun, install, build action)
Build->>Artifact: upload action dist artifact
Workflow->>Test: trigger tests depending on build
Test->>Artifact: download action artifact
Test->>Test: setup Node, install compressors, run tests using downloaded action
Test-->>Workflow: test results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 35 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/action/__tests__/action.test.ts">
<violation number="1" location="packages/action/__tests__/action.test.ts:72">
P2: These threshold tests re-implement the logic inline instead of testing the actual implementation in `src/index.ts`. If the real implementation changes or has a bug, these tests won't detect it. Consider extracting the threshold logic into a separate testable function (e.g., `shouldFailThreshold(reduction, options)`) and importing/testing that function here.</violation>
</file>
<file name="packages/action/README.md">
<violation number="1" location="packages/action/README.md:20">
P2: Documentation shows incorrect action path. According to the PR description, users should use `srod/node-minify@v1` (or `@main`), not the internal package path `srod/node-minify/packages/action@main`. This inconsistency will confuse users following the migration guide.</violation>
</file>
<file name="packages/action/src/reporters/comment.ts">
<violation number="1" location="packages/action/src/reporters/comment.ts:34">
P2: Missing pagination when fetching PR comments. If a PR has more than 30 comments, the existing node-minify comment may not be found, causing duplicate comments to be created instead of updating the existing one. Use `octokit.paginate()` to fetch all comments.</violation>
</file>
<file name="scripts/check-published.ts">
<violation number="1" location="scripts/check-published.ts:81">
P2: Unhandled promise rejection: `main()` returns a Promise but errors are not caught. Add `.catch()` to handle potential errors (e.g., JSON parse failures, filesystem errors).</violation>
</file>
<file name="packages/action/src/reporters/annotations.ts">
<violation number="1" location="packages/action/src/reporters/annotations.ts:15">
P2: Logic bug: when `file.reduction < 0`, both the warning (line 15) and error (line 29) fire. The warning message "Very low compression ratio" is misleading for files that got larger. Restructure to check for negative reduction first to avoid duplicate/confusing annotations.</violation>
</file>
<file name=".github/workflows/release-action.yml">
<violation number="1" location=".github/workflows/release-action.yml:46">
P1: Shell injection vulnerability: `github.event.inputs.tag` is directly interpolated into the shell script. This is a security anti-pattern as the expression is evaluated before shell execution. Use environment variables to safely pass the values:
```yaml
env:
INPUT_TAG: ${{ github.event.inputs.tag }}
REF_NAME: ${{ github.ref_name }}
run: |
if [ -n "$INPUT_TAG" ]; then
VERSION="$INPUT_TAG"
...
```</violation>
</file>
<file name="packages/action/src/index.ts">
<violation number="1" location="packages/action/src/index.ts:42">
P2: Confusing error message: when `totalReduction` is negative (indicating size increase), the message displays "-X.Y% increase" instead of "X.Y% increase". Use `Math.abs(result.totalReduction)` for clearer messaging.</violation>
</file>
<file name="packages/action/src/reporters/summary.ts">
<violation number="1" location="packages/action/src/reporters/summary.ts:68">
P2: Truthy check on `c.reduction` will incorrectly show "-" when reduction is exactly 0%. Use explicit null check instead: `c.reduction != null`.</violation>
<violation number="2" location="packages/action/src/reporters/summary.ts:70">
P2: Truthy check on `c.timeMs` will incorrectly show "-" when time is exactly 0ms. Use explicit null check instead: `c.timeMs != null`.</violation>
</file>
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:298">
P2: This references the legacy action location. The new action introduced by this PR is at `packages/action/` with a root `action.yml` enabling direct usage.</violation>
<violation number="2" location="AGENTS.md:318">
P2: The description says "composite steps" but the root `action.yml` uses `runs.using: "node20"` making it a Node.js action, not a composite action.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/node-minify/minify.ts (1)
148-166: UsegetFilesizeGzippedRawfor direct byte values instead of parsing formatted strings.Lines 152-165 retrieve a formatted gzip size string from
getFilesizeGzippedInBytes, then parse it back to extract the numeric byte value. This is inefficient and error-prone.Based on the PR context, a
getFilesizeGzippedRawutility appears to exist in@node-minify/utilsthat returns raw bytes directly. Use it instead.♻️ Refactor to use getFilesizeGzippedRaw
Update the import:
import { getFilesizeGzippedInBytes, + getFilesizeGzippedRaw, resolveCompressor, } from "@node-minify/utils";Replace the gzip calculation:
let gzipSize: number | null = null; let gzipSizeFormatted: string | null = null; if (includeGzip) { - const gzipResult = await getFilesizeGzippedInBytes(outputPath); - const match = gzipResult.match(/([\d.]+)\s*(\w+)/); - if (match) { - const value = Number.parseFloat(match[1]); - const unit = match[2]; - const multipliers: Record<string, number> = { - B: 1, - kB: 1024, - MB: 1024 * 1024, - GB: 1024 * 1024 * 1024, - }; - gzipSize = Math.round(value * (multipliers[unit] || 1)); - gzipSizeFormatted = gzipResult; - } + gzipSize = await getFilesizeGzippedRaw(outputPath); + gzipSizeFormatted = await getFilesizeGzippedInBytes(outputPath); }
🤖 Fix all issues with AI agents
In @action.yml:
- Line 48: The default benchmark-compressors list in action.yml includes "oxc"
but the package @node-minify/oxc is not declared in
packages/action/package.json, causing runtime import failures when
resolveCompressor tries to load it; fix by either adding "@node-minify/oxc" to
the dependencies in packages/action/package.json (so resolveCompressor can
import it) or remove "oxc" from the default: "terser,esbuild,swc,oxc" value in
action.yml so the action no longer references an undeclared dependency.
In @packages/action/README.md:
- Line 20: The README usage examples reference the subpath "uses:
srod/node-minify/packages/action@main" which will not work for a
Marketplace-published action; update every occurrence of that exact string in
the README (the three usage examples) to "uses: srod/node-minify@v1" so the
examples point to the root action.yml entrypoint.
In @packages/action/src/outputs.ts:
- Around line 17-23: The condition only checks result.files[0]?.gzipSize and can
miss gzip data on other files; change the guard to detect if any file has gzip
data (e.g., use result.files.some(f => f.gzipSize != null) or equivalent) before
computing totalGzip, then keep using the existing reduce and
setOutput("gzip-size", totalGzip) so gzip-size is emitted when any file contains
gzipSize.
In @packages/action/src/reporters/annotations.ts:
- Around line 14-36: The logic emits both a warning/notice and an error when
file.reduction is negative; change the ordering in the loop over result.files so
the negative case is checked first (e.g., test file.reduction < 0 and call
error(...) before any VERY_LOW_REDUCTION_THRESHOLD/LOW_REDUCTION_THRESHOLD
checks), or make the negative check an else-if branch to ensure error() takes
precedence over warning()/notice() for the same file; refer to the loop over
result.files and the symbols file.reduction, error(), warning(), notice(),
VERY_LOW_REDUCTION_THRESHOLD, and LOW_REDUCTION_THRESHOLD.
In @scripts/check-published.ts:
- Around line 16-39: In checkPublished, prevent shell injection by validating
packageName and version against strict patterns (e.g., npm package name rules
and semver) before use, and replace execSync template strings with a safe
execFileSync/child_process.spawnSync call that passes arguments as an array
(e.g., call 'npm' with ['view', packageName, 'version', '--json'] and ['view',
`${packageName}@${version}`, 'version', '--json']) so no shell interpolation
occurs; also change the catch handler from catch (_e) to catch (e) and follow
the guideline by checking if (e instanceof Error) before using or logging e,
then return the same { exists: false, publishedVersion: false } result.
🧹 Nitpick comments (17)
scripts/check-published.ts (2)
7-14: Consider using async file operations for consistency.While synchronous file operations are acceptable in script contexts, the coding guidelines encourage using file operation utilities from
@node-minify/utilswhere applicable. Consider refactoring to async operations for consistency with the rest of the codebase.As per coding guidelines, use file operation utilities from
@node-minify/utils.
41-79: Consider async file utilities and parallel processing.The function has a few areas for improvement:
- Line 49 uses
readFileSyncinstead of thereadFileutility from@node-minify/utilsas recommended by coding guidelines.- Line 49 performs
JSON.parsewithout error handling, which could throw on malformed files.- Lines 47-55 process packages sequentially. Per coding guidelines, consider using
Promise.allfor parallel npm queries to improve performance.♻️ Proposed refactor
+import { readFile } from "@node-minify/utils"; + async function main() { const dirs = getPackageDirs(); - const results = []; console.log("Checking packages..."); - for (const dir of dirs) { + const checks = dirs.map(async (dir) => { const pkgPath = join(PACKAGES_DIR, dir, "package.json"); - const pkg = JSON.parse(readFileSync(pkgPath, "utf-8")); + try { + const content = await readFile(pkgPath); + const pkg = JSON.parse(content); - if (pkg.private) continue; + if (pkg.private) return null; - const status = await checkPublished(pkg.name, pkg.version); - results.push({ name: pkg.name, version: pkg.version, ...status }); - } + const status = await checkPublished(pkg.name, pkg.version); + return { name: pkg.name, version: pkg.version, ...status }; + } catch (err) { + if (err instanceof Error) { + console.error(`Error reading ${pkgPath}: ${err.message}`); + } + return null; + } + }); + + const results = (await Promise.all(checks)).filter((r) => r !== null); const missing = results.filter((r) => !r.exists); const versionMismatch = results.filter( (r) => r.exists && !r.publishedVersion ); if (missing.length > 0) { console.log("\nPackages NOT FOUND on NPM (New packages):"); for (const p of missing) { console.log(`${p.name}@${p.version}`); } } if (versionMismatch.length > 0) { console.log("\nPackages with unpublished versions (Update needed):"); for (const p of versionMismatch) { console.log(`${p.name}@${p.version}`); } } if (missing.length === 0 && versionMismatch.length === 0) { console.log("\nAll packages are up to date on NPM."); } }As per coding guidelines, use file operation utilities from
@node-minify/utilsandPromise.allfor parallel operations where all must succeed.action.yml (1)
65-67: Consider clarifying whengithub-tokenis required.The
github-tokeninput description states it's for "PR comments" but doesn't specify that it's required whenreport-pr-commentis enabled. Consider adding a note like: "Required when report-pr-comment is true. Use ${{ secrets.GITHUB_TOKEN }}".packages/action/src/index.ts (1)
32-34: Verify github-token is provided before posting PR comment.The code checks for
context.payload.pull_requestbut doesn't verify thatinputs.githubTokenis provided. WhilepostPRCommentmight handle this internally, an early check here would provide clearer error messages to users who enable PR comments without providing a token.🔍 Suggested guard clause
- if (inputs.reportPRComment && context.payload.pull_request) { + if (inputs.reportPRComment && context.payload.pull_request && inputs.githubToken) { await postPRComment(result, inputs.githubToken); }packages/action/__tests__/action.test.ts (1)
1-96: Consider adding integration tests for actual minification behavior.The current tests validate types and threshold logic, which is a good start. However, consider adding tests that exercise the actual minification workflow, including:
- Testing minify function with real compressors
- Validating reporter outputs (summary, annotations, PR comments)
- Testing error handling for invalid inputs or failed minification
Note: The PR mentions workflow-level integration tests in test-action.yml, which may already cover these scenarios.
.github/workflows/release-action.yml (1)
34-41: Potential redundancy in build steps.Lines 36-37 run both
build:depsand then explicitly build@node-minify/core. Ifbuild:depsalready builds all dependencies including core, the second command may be redundant.♻️ Simplify if build:deps already handles core
- name: Build dependencies run: | bun run build:deps - bun run --filter '@node-minify/core' build.github/actions/node-minify/minify.ts (1)
112-114: Consider extracting the hard-coded compressor list to a shared constant.The list of compressors requiring a
typeinput is hard-coded here. If this logic is needed elsewhere or changes frequently, consider centralizing it in@node-minify/utilsor a constants file.packages/action/package.json (1)
42-44: Clarify which build script is used for the release.Two build scripts are defined:
- Line 43:
builduses Bun bundler- Line 44:
build:nccuses @vercel/nccBased on the release workflow (.github/workflows/release-action.yml line 40), the
buildscript (Bun) appears to be the primary bundler. Consider documenting this choice or removing the unusedbuild:nccscript to avoid confusion.packages/action/src/reporters/comment.ts (2)
34-58: Consider adding error handling for GitHub API calls.The Octokit API calls for listing, updating, and creating comments lack error handling. If the GitHub API returns an error (rate limiting, network issues, permission denied), the action will fail with an unhandled exception.
♻️ Suggested improvement
+ try { const { data: comments } = await octokit.rest.issues.listComments({ owner, repo, issue_number: prNumber, }); const existingComment = comments.find((c) => c.body?.includes(COMMENT_TAG)); if (existingComment) { await octokit.rest.issues.updateComment({ owner, repo, comment_id: existingComment.id, body, }); info(`Updated existing PR comment #${existingComment.id}`); } else { const { data: newComment } = await octokit.rest.issues.createComment({ owner, repo, issue_number: prNumber, body, }); info(`Created new PR comment #${newComment.id}`); } + } catch (err) { + if (err instanceof Error) { + warning(`Failed to post PR comment: ${err.message}`); + } + }
34-38: Pagination not handled for comment listing.The
listCommentsAPI call uses default pagination (30 items per page). For PRs with many comments, the node-minify comment may not be found if it's beyond the first page, resulting in duplicate comments being created.♻️ Consider using paginate helper
- const { data: comments } = await octokit.rest.issues.listComments({ - owner, - repo, - issue_number: prNumber, - }); + const comments = await octokit.paginate(octokit.rest.issues.listComments, { + owner, + repo, + issue_number: prNumber, + });packages/action/src/minify.ts (1)
13-16: Consider usinggetFilesizeInBytesfrom@node-minify/utilsinstead of custom helper.The
getFileSizehelper duplicates functionality already available in@node-minify/utils. Per the coding guidelines, file operation utilities from@node-minify/utilsshould be preferred.♻️ Suggested change
-import { stat } from "node:fs/promises"; import { resolve } from "node:path"; import { minify } from "@node-minify/core"; -import { getFilesizeGzippedRaw, resolveCompressor } from "@node-minify/utils"; +import { getFilesizeGzippedRaw, getFilesizeInBytes, resolveCompressor } from "@node-minify/utils"; import type { ActionInputs, FileResult, MinifyResult } from "./types.ts"; -async function getFileSize(filePath: string): Promise<number> { - const stats = await stat(filePath); - return stats.size; -}Then replace
getFileSize(inputPath)withgetFilesizeInBytes(inputPath)at lines 24 and 42.Based on learnings, file operation utilities from
@node-minify/utilsshould be used.packages/action/src/inputs.ts (1)
81-81: Misleading alias:validateJavaCompressordoesn't validate Java compressors specifically.The alias
validateJavaCompressorpoints tovalidateCompressor, which only checks for deprecation and built-in status. It doesn't perform any Java-specific validation (e.g., checking if Java is installed, or if the compressor is inJAVA_COMPRESSORS). Consider removing the alias or implementing actual Java validation.packages/action/src/reporters/summary.ts (1)
46-55: Consider improving failed compressor row layout.For failed compressors, the error message is placed in the "Time" column (index 4), which may confuse readers expecting timing data. Consider spanning the error across columns or using a dedicated layout.
♻️ Alternative: clearer error display
if (!c.success) { return [ { data: c.compressor }, - { data: "❌ Failed" }, - { data: "-" }, - { data: "-" }, - { data: c.error || "Unknown error" }, + { data: `❌ ${c.error || "Failed"}` }, + { data: "-" }, + { data: "-" }, + { data: "-" }, ]; }AGENTS.md (2)
315-320: Add blank lines around the table for proper Markdown formatting.Per markdownlint, tables should be surrounded by blank lines for consistent rendering.
♻️ Suggested fix
### Files + | File | Purpose | |------|---------| | `action.yml` | Action definition, inputs/outputs, composite steps | | `minify.ts` | Bun script that runs minification, writes GitHub outputs | +
296-308: Update action usage example to reflect new Marketplace action.The documentation still references the old composite action path (
.github/actions/node-minify@main), but the PR introduces a new Marketplace-publishable action at the root. Consider updating to the new usage patternsrod/node-minify@v1as mentioned in the PR objectives.♻️ Update to new action reference
### Usage ```yaml -- uses: srod/node-minify/.github/actions/node-minify@main +- uses: srod/node-minify@v1 with: input: "src/app.js" output: "dist/app.min.js" compressor: "terser" # or esbuild, swc, lightningcss, etc. type: "js" # required for esbuild, lightningcss, yui</details> </blockquote></details> <details> <summary>.github/workflows/test-action.yml (2)</summary><blockquote> `66-74`: **Package installation approach works but is fragile.** The approach of installing packages to a temp directory and copying to `$GITHUB_WORKSPACE/node_modules` works, but: 1. It duplicates the compressor dependencies already bundled in the action 2. Copying with `cp -r` may fail silently if there are permission issues Consider documenting why this is needed (dynamic imports at runtime require unbundled packages) or exploring alternatives. --- `217-219`: **Potential shell injection with direct output interpolation.** Using `${{ steps.minify-css.outputs.* }}` directly in `echo` commands is generally safe for numeric outputs, but if the output ever contains shell metacharacters, it could cause issues. The JS test job (lines 103-113) correctly uses environment variables for this purpose. <details> <summary>♻️ Use environment variables for consistency</summary> ```diff - name: Verify CSS output + env: + ORIGINAL_SIZE: ${{ steps.minify-css.outputs.original-size }} + MINIFIED_SIZE: ${{ steps.minify-css.outputs.minified-size }} + REDUCTION_PERCENT: ${{ steps.minify-css.outputs.reduction-percent }} run: | - echo "Original: ${{ steps.minify-css.outputs.original-size }} bytes" - echo "Minified: ${{ steps.minify-css.outputs.minified-size }} bytes" - echo "Reduction: ${{ steps.minify-css.outputs.reduction-percent }}%" + echo "Original: $ORIGINAL_SIZE bytes" + echo "Minified: $MINIFIED_SIZE bytes" + echo "Reduction: $REDUCTION_PERCENT%" - if [ "$(echo '${{ steps.minify-css.outputs.reduction-percent }} > 0' | bc -l)" -ne 1 ]; then + if [ "$(echo "$REDUCTION_PERCENT > 0" | bc -l)" -ne 1 ]; then echo "::error::CSS reduction percentage should be > 0" exit 1 fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.github/actions/node-minify/README.md.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts.github/workflows/release-action.yml.github/workflows/test-action.yml.jules/palette.mdAGENTS.mdaction.ymldocs/AGENTS.mddocs/src/consts.tsdocs/src/content/docs/benchmark.mddocs/src/content/docs/cli.mddocs/src/content/docs/github-action.mdpackages/action/README.mdpackages/action/__tests__/action.test.tspackages/action/action.ymlpackages/action/package.jsonpackages/action/src/index.d.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/src/minify.tspackages/action/src/outputs.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/comment.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/tsconfig.jsonpackages/action/vitest.config.tspackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/utils/src/compressor-resolver.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/utils/src/index.tsscripts/check-published.ts
💤 Files with no reviewable changes (1)
- .jules/palette.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/outputs.tspackages/action/src/index.tspackages/action/src/reporters/comment.tspackages/action/src/index.d.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/benchmark/src/runner.tspackages/action/src/reporters/summary.tspackages/action/vitest.config.tspackages/action/src/minify.tspackages/utils/src/compressor-resolver.tspackages/utils/src/index.tspackages/action/src/inputs.tspackages/action/src/types.tspackages/action/__tests__/action.test.tsscripts/check-published.tspackages/action/src/reporters/annotations.tsdocs/src/consts.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/outputs.tspackages/action/src/index.tspackages/action/src/reporters/comment.tspackages/action/src/index.d.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/benchmark/src/runner.tspackages/action/src/reporters/summary.tspackages/action/vitest.config.tspackages/action/src/minify.tspackages/utils/src/compressor-resolver.tspackages/utils/src/index.tspackages/action/src/inputs.tspackages/action/src/types.tspackages/action/__tests__/action.test.tsscripts/check-published.tspackages/action/src/reporters/annotations.tsdocs/src/consts.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/action/src/index.tspackages/utils/src/index.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/action/__tests__/action.test.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
docs/src/content/docs/github-action.mddocs/src/content/docs/cli.mdpackages/action/src/outputs.tspackages/action/src/index.ts.github/actions/node-minify/README.mdpackages/action/src/index.d.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/action/src/reporters/summary.tspackages/action/action.ymlpackages/action/src/minify.tspackages/utils/src/compressor-resolver.tspackages/utils/src/index.tsdocs/src/content/docs/benchmark.mdpackages/action/src/inputs.tsaction.ymlpackages/action/src/types.ts.github/actions/node-minify/action.ymlpackages/action/__tests__/action.test.tspackages/action/README.md.github/actions/node-minify/minify.tspackages/action/src/reporters/annotations.tsdocs/src/consts.tsAGENTS.md.github/workflows/test-action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
docs/src/content/docs/github-action.mdpackages/action/src/index.ts.github/actions/node-minify/README.mdpackages/action/src/index.d.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/benchmark/README.mdpackages/action/action.ymlpackages/action/src/minify.tspackages/utils/src/index.tspackages/action/src/inputs.tsaction.ymlpackages/action/src/types.tspackages/action/__tests__/action.test.tspackages/action/README.md.github/actions/node-minify/minify.tspackages/action/src/reporters/annotations.tsAGENTS.md.github/workflows/test-action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
docs/src/content/docs/github-action.mddocs/src/content/docs/cli.mdpackages/action/src/outputs.tspackages/action/src/index.ts.github/actions/node-minify/README.mdpackages/action/src/index.d.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/action/src/reporters/summary.tspackages/action/action.ymlpackages/action/src/minify.tspackages/utils/src/compressor-resolver.tspackages/utils/src/index.tsdocs/src/content/docs/benchmark.mdpackages/action/src/inputs.tsaction.ymlpackages/action/src/types.ts.github/actions/node-minify/minify.tsdocs/src/consts.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
docs/src/content/docs/cli.mdpackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/action/src/minify.tspackages/action/tsconfig.jsonpackages/utils/src/index.tsdocs/src/content/docs/benchmark.mdpackages/action/__tests__/action.test.tsAGENTS.md.github/workflows/test-action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
docs/src/content/docs/cli.md.github/actions/node-minify/README.mdpackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/action/action.ymldocs/src/content/docs/benchmark.md.github/actions/node-minify/minify.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
packages/action/src/index.tspackages/action/src/index.d.tspackages/action/src/minify.tspackages/action/tsconfig.jsonpackages/action/package.jsonpackages/action/src/reporters/annotations.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Applied to files:
.github/actions/node-minify/README.mdpackages/action/src/index.d.ts.github/actions/node-minify/action.ymlpackages/action/src/reporters/annotations.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Use `type` keyword for type-only imports in TypeScript
Applied to files:
packages/action/src/index.d.tspackages/action/tsconfig.jsonAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Include `.ts` file extension in local imports (e.g., `./setup.ts`)
Applied to files:
packages/action/src/index.d.tspackages/action/tsconfig.jsonAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: All packages follow standard structure: `src/index.ts`, `__tests__/`, `package.json`, `tsconfig.json`, built with `tsdown`
Applied to files:
packages/action/src/index.d.tspackages/action/tsconfig.jsonAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `node:` prefix for Node.js built-in imports (e.g., `node:child_process`)
Applied to files:
packages/action/src/index.d.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
packages/action/vitest.config.tspackages/action/__tests__/action.test.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Use PascalCase for type and interface names
Applied to files:
packages/action/tsconfig.json
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
packages/utils/src/index.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Use Bun as the package manager and runtime for this repository (Node.js >=20.0.0, Bun 1.3.5+)
Applied to files:
scripts/check-published.tsAGENTS.md
🧬 Code graph analysis (6)
packages/action/src/outputs.ts (1)
packages/action/src/types.ts (2)
MinifyResult(35-42)BenchmarkResult(54-61)
packages/action/src/reporters/comment.ts (2)
packages/action/src/types.ts (1)
MinifyResult(35-42)packages/utils/src/index.ts (1)
prettyBytes(48-48)
packages/action/src/reporters/summary.ts (1)
packages/action/src/types.ts (2)
MinifyResult(35-42)BenchmarkResult(54-61)
packages/action/src/minify.ts (5)
packages/action/src/types.ts (3)
ActionInputs(7-23)MinifyResult(35-42)FileResult(25-33)packages/utils/src/compressor-resolver.ts (1)
resolveCompressor(164-258)packages/utils/src/index.ts (2)
resolveCompressor(52-52)getFilesizeGzippedRaw(41-41)packages/core/src/index.ts (1)
minify(21-27)packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(76-86)
packages/action/__tests__/action.test.ts (1)
packages/action/src/types.ts (3)
ActionInputs(7-23)FileResult(25-33)MinifyResult(35-42)
packages/action/src/reporters/annotations.ts (1)
packages/action/src/types.ts (1)
MinifyResult(35-42)
🪛 actionlint (1.7.10)
.github/workflows/test-action.yml
95-95: file "packages/action/dist/index.js" does not exist in "/home/jailuser/git". it is specified at "main" key in "runs" section in "node-minify" action
(action)
🪛 LanguageTool
AGENTS.md
[uncategorized] ~298-~298: The official name of this software platform is spelled with a capital “H”.
Context: ...ry includes a reusable GitHub Action at .github/actions/node-minify/ for CI/CD minific...
(GITHUB)
[uncategorized] ~323-~323: The official name of this software platform is spelled with a capital “H”.
Context: ...tputs | ## CI/CD Workflows Located in .github/workflows/: | Workflow | Trigger | Pu...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
316-316: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Greptile Review
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (windows-latest, 24.x)
🔇 Additional comments (31)
scripts/check-published.ts (1)
1-5: LGTM!The imports correctly use the
node:prefix for Node.js built-in modules, following the coding guidelines.packages/action/tsconfig.json (1)
1-8: LGTM!Standard TypeScript configuration for a monorepo package. The paths and structure are correct.
docs/src/consts.ts (1)
15-15: LGTM!The sidebar entry follows the existing pattern and correctly links to the new GitHub Action documentation.
packages/utils/src/compressor-resolver.ts (1)
32-32: LGTM!The minify-html compressor mapping follows the established pattern and naming conventions correctly.
.github/actions/node-minify/README.md (3)
1-16: LGTM!Clear deprecation notice with a straightforward migration path and compelling reasons to upgrade.
17-39: LGTM!Migration guide clearly shows the straightforward path from the deprecated action to the new bundled action with minimal changes required.
41-83: LGTM!Legacy documentation is appropriately preserved for existing users while being clearly marked as deprecated.
packages/action/vitest.config.ts (1)
4-5: Reduce test timeout from 30 seconds to 5-10 seconds.The test file contains only 6 simple unit tests for type validation and threshold logic with no async operations, file I/O, or real minification. These tests should complete in milliseconds. A 30-second timeout is excessive and can mask performance regressions. Use a more reasonable timeout (e.g., 5-10 seconds) that matches the actual test complexity.
⛔ Skipped due to learnings
Learnt from: CR Repo: srod/node-minify PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-09T07:41:29.707Z Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`Learnt from: CR Repo: srod/node-minify PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-09T07:41:29.707Z Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustionpackages/utils/src/index.ts (1)
16-19: LGTM! Clean API extension for raw gzip size.The addition of
getFilesizeGzippedRawfollows existing patterns and provides the raw numeric API needed by the action workflow. The import and export are consistent with the codebase style.Also applies to: 41-41
action.yml (1)
1-87: LGTM! Comprehensive GitHub Action manifest.The action manifest is well-structured with clear inputs, outputs, and proper Node20 runtime configuration. The bundled entry point at
packages/action/dist/index.jsaligns with the PR's bundling strategy..github/actions/node-minify/action.yml (2)
1-2: Excellent deprecation strategy with clear migration path.The name, description, and branding color changes effectively communicate the deprecated status. The migration guidance to
srod/node-minify@mainis clear and actionable.Also applies to: 6-6
70-73: Good visibility for deprecation warnings.The runtime warning annotation and job summary note ensure users are informed about the deprecation at multiple touchpoints. This follows best practices for action deprecation.
Also applies to: 140-141
packages/action/src/types.ts (1)
1-69: LGTM! Well-structured type definitions.The interfaces are comprehensive and properly typed, covering all aspects of the action workflow including inputs, results, benchmarking, and comparisons. The copyright header and naming conventions follow project standards.
packages/action/src/index.d.ts (1)
1-7: LGTM! Proper declaration file pattern.The declaration file correctly re-exports types from
./types.ts, making them available at the package root. The copyright header and file extension in the import follow project conventions.packages/benchmark/src/runner.ts (1)
65-70: LGTM! Clean addition of oxc to default compressors.The "oxc" compressor is correctly added to the default list, maintaining consistent formatting and alignment with the documentation updates across the codebase.
docs/src/content/docs/cli.md (1)
81-81: Documentation correctly updated to reflect oxc addition.The CLI documentation examples and defaults table are properly synchronized with the code changes.
Also applies to: 108-108
docs/src/content/docs/benchmark.md (1)
31-31: Documentation comprehensively updated.All benchmark documentation sections (CLI examples, programmatic API, and options reference) correctly reflect the addition of "oxc" to the default compressors.
Also applies to: 87-87, 127-127
packages/benchmark/README.md (1)
34-34: Package README accurately reflects the updated defaults.All references to default compressors in the benchmark package README are correctly updated to include "oxc".
Also applies to: 49-49, 69-69
packages/action/README.md (1)
78-78: Compressor documentation includes oxc.The available compressors list correctly includes "oxc" alongside other JavaScript minifiers, consistent with the broader changes in this PR.
packages/action/src/index.ts (1)
28-55: Confirm reporting before failure is intentional.The current flow generates all reports (summary, PR comment, annotations) before checking failure conditions (
fail-on-increase,min-reduction). This means users see the compression results even when the action fails, which provides good visibility but might be unexpected behavior.Is this the intended UX? If so, consider adding a comment explaining this design choice. If not, the failure checks could be moved before reporting.
packages/utils/src/getFilesizeGzippedInBytes.ts (2)
12-45: Well-structured helper extraction.The
getGzipSizeinternal helper effectively consolidates file validation and streaming logic, eliminating duplication betweengetFilesizeGzippedInBytesand the newgetFilesizeGzippedRawfunction. The error handling is consistent and provides clear messages.
68-86: LGTM: New public API for raw gzip sizes.The
getFilesizeGzippedRawfunction provides a useful raw byte count interface that complements the existing formatted string output. The implementation correctly reuses the internalgetGzipSizehelper and maintains consistent error handling..github/workflows/release-action.yml (2)
64-71: Action.yml configuration verified. Root action.yml exists and correctly referencespackages/action/dist/index.jsas the entry point for the bundled action.
26-29: Bun version 1.3.5 is already the latest stable release. No update required.Likely an incorrect or invalid review comment.
packages/action/src/minify.ts (1)
18-70: LGTM! Solid implementation of the minification workflow.The function correctly:
- Resolves paths relative to working directory
- Uses
resolveCompressorfor dynamic compressor resolution (per learnings)- Conditionally spreads
typeandoptionsonly when present- Handles division by zero in reduction calculation
- Optionally computes gzip size
packages/action/src/inputs.ts (2)
23-65: LGTM! Well-structured input parsing with proper validation.The implementation correctly:
- Validates required
typeinput for specific compressors- Parses JSON options with proper error handling
- Provides sensible defaults for benchmark compressors including
oxc- Falls back to
GITHUB_TOKENenvironment variable when not provided as input
12-12:JAVA_COMPRESSORSis exported and is not unused.The constant is explicitly exported on line 83 as part of the public API, making it available for external use. No changes needed.
Likely an incorrect or invalid review comment.
packages/action/src/reporters/summary.ts (1)
11-41: LGTM! Clean summary generation with proper table formatting.The implementation correctly builds the summary table with all relevant metrics and uses the chained API pattern for
@actions/coresummary.AGENTS.md (1)
1-340: LGTM! Comprehensive documentation update.The AGENTS.md file provides excellent guidance with updated sections for:
- Image file support and new compressors (minify-html, sharp, svgo, imagemin)
- Extended
CompressorResulttype withbuffer?andoutputs?- New
ensureStringContentutility usage- CLI usage examples and benchmark formats
- Anti-patterns section for clear "do nots"
- GitHub Action and CI/CD workflow documentation
.github/workflows/test-action.yml (2)
18-46: LGTM! Well-structured build job with artifact sharing.The build-action job correctly:
- Builds dependencies in the right order (
build:deps, then@node-minify/core)- Builds the action package
- Uploads the dist folder as an artifact for downstream jobs
93-99: Action reference updated correctly for Marketplace usage.The change from
uses: ./.github/actions/node-minifytouses: ./correctly references the rootaction.yml, enabling thesrod/node-minify@v1usage pattern for Marketplace publication.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces a proper GitHub Action package for node-minify that can be published to the Marketplace. The implementation includes comprehensive features like PR comments, job summaries, file annotations, and threshold enforcement. The code quality is generally good with proper error handling and modular architecture.
However, there are two critical issues that will prevent the action from working:
-
Release workflow doesn't commit dist files: The
packages/action/dist/directory is gitignored, but the release workflow never commits the built files before creating tags. When users runuses: srod/node-minify@v1, GitHub checks out a commit without dist files, causing immediate failure. -
Benchmark feature incomplete: The
benchmarkinput is declared in action.yml and parsed, but the actual benchmark logic is never executed. Users settingbenchmark: truewon't get results, and thebenchmark-winneroutput will never be set. ThegenerateBenchmarkSummaryandsetBenchmarkOutputsfunctions exist but are orphaned.
Additionally, there are documentation inconsistencies where three different action reference patterns are shown (@v1, @main, and /packages/action@main).
The core minification functionality, reporters, and utils changes are solid and well-implemented.
Confidence Score: 1/5
- This PR cannot be safely merged as the action will fail immediately when users try to use it
- Two critical blockers prevent this from working: (1) dist files aren't committed but the action requires them, causing immediate module not found errors, and (2) the advertised benchmark feature is declared but completely unimplemented. These are not minor bugs but fundamental issues that break core functionality.
- .github/workflows/release-action.yml must commit dist files before tagging, and packages/action/src/index.ts needs benchmark implementation or the feature should be removed from inputs/outputs
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .github/workflows/release-action.yml | 1/5 | CRITICAL: Builds dist files but never commits them before creating tags, causing action to fail when used |
| packages/action/src/index.ts | 2/5 | CRITICAL: Benchmark feature declared in inputs/outputs but not implemented; missing runBenchmark logic |
| action.yml | 4/5 | Root action metadata properly configured; points to dist/index.js which needs to be committed |
| packages/action/action.yml | 4/5 | Package action metadata with comprehensive inputs; benchmark inputs need implementation |
| packages/action/README.md | 3/5 | Documentation uses incorrect action path; should be srod/node-minify@v1 not packages/action@main |
| .github/actions/node-minify/README.md | 4/5 | Deprecation notice properly warns users; migration path shows @main instead of @v1 |
Sequence Diagram
sequenceDiagram
participant User as GitHub Workflow
participant Action as Action Entry (index.ts)
participant Inputs as Input Parser
participant Minify as Minification Engine
participant Reporters as Reporters
participant GitHub as GitHub API
User->>Action: Trigger action with inputs
Action->>Inputs: parseInputs()
Inputs-->>Action: ActionInputs
Action->>Inputs: validateJavaCompressor()
Note over Inputs: Check for deprecated compressors
Action->>Minify: runMinification(inputs)
Minify->>Minify: resolveCompressor()
Minify->>Minify: minify() via @node-minify/core
Minify->>Minify: Calculate sizes & gzip
Minify-->>Action: MinifyResult
Action->>Action: setMinifyOutputs()
Note over Action: Set GitHub Action outputs
alt report-summary enabled
Action->>Reporters: generateSummary()
Reporters->>GitHub: Write job summary
end
alt report-pr-comment enabled AND is PR
Action->>Reporters: postPRComment()
Reporters->>GitHub: Create/update PR comment
end
alt report-annotations enabled
Action->>Reporters: addAnnotations()
Note over Reporters: Add warnings for low compression
end
alt failOnIncrease AND size increased
Action->>Action: setFailed()
end
alt minReduction threshold not met
Action->>Action: setFailed()
end
Action-->>User: Complete (success/failure)
| echo "version=$VERSION" >> $GITHUB_OUTPUT | ||
| echo "major=$(echo $VERSION | cut -d. -f1)" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Update major version tag | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| # Force update the major version tag (e.g., v1) |
There was a problem hiding this comment.
The release workflow builds the dist files but never commits them to the repository. Since packages/action/dist/ is gitignored (root .gitignore line 7), when users run uses: srod/node-minify@v1, GitHub will checkout the v1 tag which points to a commit WITHOUT the dist files, causing the action to fail with "Cannot find module" errors.
Solution: Before creating/updating the tag, you need to commit the built dist files:
- name: Build action
run: bun run build
working-directory: packages/action
- name: Commit built files
run: |
git add packages/action/dist/
git commit -m "Build action for release ${{ steps.version.outputs.version }}"
git push
- name: Update major version tag
run: |
# ... existing tag codeAlternatively, you could create a separate release branch or remove dist from .gitignore and check it in (common for GitHub Actions).
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-action.yml
Line: 54:62
Comment:
The release workflow builds the dist files but never commits them to the repository. Since `packages/action/dist/` is gitignored (root .gitignore line 7), when users run `uses: srod/node-minify@v1`, GitHub will checkout the v1 tag which points to a commit WITHOUT the dist files, causing the action to fail with "Cannot find module" errors.
**Solution**: Before creating/updating the tag, you need to commit the built dist files:
```yaml
- name: Build action
run: bun run build
working-directory: packages/action
- name: Commit built files
run: |
git add packages/action/dist/
git commit -m "Build action for release ${{ steps.version.outputs.version }}"
git push
- name: Update major version tag
run: |
# ... existing tag code
```
Alternatively, you could create a separate release branch or remove dist from .gitignore and check it in (common for GitHub Actions).
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| ### Basic Minification | ||
|
|
||
| ```yaml | ||
| - name: Minify JavaScript | ||
| uses: srod/node-minify@v1 |
There was a problem hiding this comment.
Documentation inconsistency: The package README shows uses: srod/node-minify/packages/action@main, but the PR description and docs/src/content/docs/github-action.md show uses: srod/node-minify@v1, and .github/actions/node-minify/README.md shows uses: srod/node-minify@main.
Since a root action.yml is being added, the correct usage is srod/node-minify@v1 (or @main for latest). The path srod/node-minify/packages/action@main won't work because there's no action.yml at that subpath.
| ### Basic Minification | |
| ```yaml | |
| - name: Minify JavaScript | |
| uses: srod/node-minify@v1 | |
| uses: srod/node-minify@v1 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/README.md
Line: 15:20
Comment:
Documentation inconsistency: The package README shows `uses: srod/node-minify/packages/action@main`, but the PR description and docs/src/content/docs/github-action.md show `uses: srod/node-minify@v1`, and .github/actions/node-minify/README.md shows `uses: srod/node-minify@main`.
Since a root action.yml is being added, the correct usage is `srod/node-minify@v1` (or `@main` for latest). The path `srod/node-minify/packages/action@main` won't work because there's no action.yml at that subpath.
```suggestion
uses: srod/node-minify@v1
```
How can I resolve this? If you propose a fix, please make it concise.| > | ||
| > ```yaml | ||
| > - uses: srod/node-minify@main | ||
| > ``` | ||
|
|
||
| ## Usage | ||
| The new action includes: |
There was a problem hiding this comment.
The migration instructions reference srod/node-minify@main, but this is inconsistent with the documentation and PR description which reference @v1. Since this is intended for Marketplace publication with versioned releases, users should be directed to a stable version tag.
| > | |
| > ```yaml | |
| > - uses: srod/node-minify@main | |
| > ``` | |
| ## Usage | |
| The new action includes: | |
| > **This action is deprecated.** Please use the new bundled action instead: | |
| > | |
| > ```yaml | |
| > - uses: srod/node-minify@v1 | |
| > ``` |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/actions/node-minify/README.md
Line: 4:9
Comment:
The migration instructions reference `srod/node-minify@main`, but this is inconsistent with the documentation and PR description which reference `@v1`. Since this is intended for Marketplace publication with versioned releases, users should be directed to a stable version tag.
```suggestion
> **This action is deprecated.** Please use the new bundled action instead:
>
> ```yaml
> - uses: srod/node-minify@v1
> ```
```
How can I resolve this? If you propose a fix, please make it concise.
packages/action/src/inputs.ts
Outdated
| } | ||
| } | ||
|
|
||
| export const validateJavaCompressor = validateCompressor; |
There was a problem hiding this comment.
The function is exported as validateJavaCompressor but it doesn't specifically validate Java compressors - it validates all compressors for deprecation warnings and checks if they're built-in. The name is misleading.
The actual Java compressor validation (checking if Java is available) is not performed here. Consider renaming to validateCompressor (which already exists as an alias) or implementing actual Java validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/inputs.ts
Line: 81:81
Comment:
The function is exported as `validateJavaCompressor` but it doesn't specifically validate Java compressors - it validates all compressors for deprecation warnings and checks if they're built-in. The name is misleading.
The actual Java compressor validation (checking if Java is available) is not performed here. Consider renaming to `validateCompressor` (which already exists as an alias) or implementing actual Java validation.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Users who set Solution: Implement the benchmark feature or remove it from the inputs/outputs until ready: if (inputs.benchmark) {
// Run benchmark with inputs.benchmarkCompressors
const benchmarkResult = await runBenchmark({
input: inputs.input,
compressors: inputs.benchmarkCompressors,
// ... other options
});
await generateBenchmarkSummary(benchmarkResult);
setBenchmarkOutputs(benchmarkResult);
}Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/action/src/index.ts
Line: 17:66
Comment:
The `benchmark` input is declared in action.yml (line 41-48) and parsed in inputs.ts (line 57), but the benchmark functionality is never implemented. The `inputs.benchmark` field is collected but never checked, and `generateBenchmarkSummary` and `setBenchmarkOutputs` functions exist but are never called.
Users who set `benchmark: true` will not get any benchmark results, and the `benchmark-winner` output promised in action.yml (line 83) will never be set.
**Solution**: Implement the benchmark feature or remove it from the inputs/outputs until ready:
```typescript
if (inputs.benchmark) {
// Run benchmark with inputs.benchmarkCompressors
const benchmarkResult = await runBenchmark({
input: inputs.input,
compressors: inputs.benchmarkCompressors,
// ... other options
});
await generateBenchmarkSummary(benchmarkResult);
setBenchmarkOutputs(benchmarkResult);
}
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/release-action.yml:
- Around line 21-25: The checkout step may pick an incorrect ref for release
events; update the ref expression used in the actions/checkout@v4 step to prefer
the release tag when available by replacing the current ref value with a
prioritized expression that uses github.event.inputs.tag first, then
github.event.release.tag_name (for published release events), and finally falls
back to github.ref so releases always build from the release tag when present.
- Around line 43-56: The "Get version" step currently uses INPUT_TAG or REF_NAME
as VERSION and derives major via cut, which can mistakenly treat branch names
like main as a tag; update the step logic to validate that the chosen value is
an actual tag (e.g., match a semver-like pattern or starts with "v" followed by
digits) before assigning VERSION, and if neither INPUT_TAG nor REF_NAME match
the tag regex, fail fast with a clear error instead of using a branch name; also
derive major by extracting the numeric major component from the validated
VERSION with a regex match rather than using cut.
In @scripts/check-published.ts:
- Around line 47-54: Wrap the file read and parse around each package in a
try/catch so a bad or missing package.json doesn't crash the whole script: when
iterating dirs, catch errors from constructing pkgPath, readFileSync(pkgPath,
"utf-8") and JSON.parse, log a warning including the package directory (dir) and
the error, and skip to the next package instead of pushing a result; keep
calling checkPublished for valid pkg objects as before and only push to results
when pkg is successfully parsed.
- Around line 16-39: The function checkPublished is declared async but only uses
synchronous calls and it interpolates packageName/version into shell commands
creating command-injection risk; change its declaration to a plain function
(remove async), validate/sanitize packageName and version against a tight regex
(e.g. restrict to allowed npm name/version characters) and then call
child_process.execFileSync (or spawnSync) instead of execSync, passing the
command and arguments as an array (e.g. execFileSync("npm", ["view",
packageName, "version", "--json"], {stdio:"pipe"}) and similarly for the
specific version), and keep the existing try/catch behavior to return the same
shape.
🧹 Nitpick comments (6)
scripts/check-published.ts (1)
7-14: Consider adding defensive check for packages directory existence.The function will throw an error if the
PACKAGES_DIRdoesn't exist or isn't readable. While this directory should exist in the normal repository structure, adding a defensive check would make the script more robust and provide a clearer error message.🛡️ Proposed defensive check
function getPackageDirs() { + if (!existsSync(PACKAGES_DIR)) { + console.error(`Error: ${PACKAGES_DIR} directory not found`); + return []; + } + return readdirSync(PACKAGES_DIR, { withFileTypes: true }) .filter((entry) => entry.isDirectory()) .filter((entry) => existsSync(join(PACKAGES_DIR, entry.name, "package.json")) ) .map((entry) => entry.name); }AGENTS.md (1)
316-320: Consider adding blank lines around the table for markdown formatting best practice.While not critical, adding blank lines before and after the table improves markdown parsing and rendering consistency across different renderers.
📝 Suggested formatting improvement
### Files + | File | Purpose | |------|---------| | `action.yml` | Action definition, inputs/outputs | | `packages/action/` | Source code for the action (built to `packages/action/dist/index.js`) | +packages/action/src/index.ts (1)
59-59: Consider handling potential unhandled promise rejection.The
run()function is invoked withoutawaitor.catch(), which could result in unhandled promise rejections if errors occur. While the internal try-catch handles most errors, it's a best practice to add a catch handler at the top level.🔧 Suggested improvement
-run(); +run().catch((error) => { + console.error("Fatal error:", error); + process.exit(1); +});Or use top-level await if your build target supports it:
-run(); +await run();packages/action/src/reporters/summary.ts (1)
67-70: Consider using strict equality operators for consistency.Lines 67-70 use loose equality (
!=) for null checks. While this works for checking bothnullandundefined, using strict equality (!==) is more explicit and consistent with modern TypeScript best practices.♻️ Suggested refactor
return [ { data: `${c.compressor}${badge}` }, - { data: c.size != null ? prettyBytes(c.size) : "-" }, - { data: c.reduction != null ? `${c.reduction.toFixed(1)}%` : "-" }, - { data: c.gzipSize != null ? prettyBytes(c.gzipSize) : "-" }, - { data: c.timeMs != null ? `${c.timeMs}ms` : "-" }, + { data: c.size !== undefined ? prettyBytes(c.size) : "-" }, + { data: c.reduction !== undefined ? `${c.reduction.toFixed(1)}%` : "-" }, + { data: c.gzipSize !== undefined ? prettyBytes(c.gzipSize) : "-" }, + { data: c.timeMs !== undefined ? `${c.timeMs}ms` : "-" }, ];.github/workflows/release-action.yml (2)
26-33: Consider pinning third-party actions by SHA (supply-chain hardening).
actions/checkout@v4,oven-sh/setup-bun@v2, andsoftprops/action-gh-release@v2are mutable tags.
57-66: Force-updating the major tag is fine, but add guardrails to prevent accidental retagging.
Givengit push --force, it’s worth ensuringMAJORis non-empty and matches^v[0-9]+$before tagging/pushing.Proposed fix
- name: Update major version tag run: | + set -euo pipefail git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" # Force update the major version tag (e.g., v1) MAJOR="${{ steps.version.outputs.major }}" + if ! echo "$MAJOR" | grep -Eq '^v[0-9]+$'; then + echo "Refusing to update unexpected major tag: '$MAJOR'" >&2 + exit 1 + fi git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}" git push origin "$MAJOR" --force
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.changeset/fluffy-eagles-sing.mdis excluded by!.changeset/**
📒 Files selected for processing (10)
.github/workflows/release-action.ymlAGENTS.mdpackages/action/README.mdpackages/action/__tests__/action.test.tspackages/action/src/checks.tspackages/action/src/index.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/comment.tspackages/action/src/reporters/summary.tsscripts/check-published.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/action/src/reporters/comment.ts
- packages/action/tests/action.test.ts
- packages/action/README.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/checks.tsscripts/check-published.tspackages/action/src/index.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/checks.tsscripts/check-published.tspackages/action/src/index.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/action/src/index.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Use Bun as the package manager and runtime for this repository (Node.js >=20.0.0, Bun 1.3.5+)
Applied to files:
scripts/check-published.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
AGENTS.mdpackages/action/src/index.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
AGENTS.mdpackages/action/src/index.tspackages/action/src/reporters/summary.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: All packages follow standard structure: `src/index.ts`, `__tests__/`, `package.json`, `tsconfig.json`, built with `tsdown`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
AGENTS.mdpackages/action/src/index.tspackages/action/src/reporters/annotations.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Applied to files:
AGENTS.mdpackages/action/src/reporters/annotations.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
AGENTS.mdpackages/action/src/index.tspackages/action/src/reporters/annotations.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `node:` prefix for Node.js built-in imports (e.g., `node:child_process`)
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Include `.ts` file extension in local imports (e.g., `./setup.ts`)
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Use `type` keyword for type-only imports in TypeScript
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
AGENTS.md
🧬 Code graph analysis (2)
packages/action/src/reporters/annotations.ts (1)
packages/action/src/types.ts (1)
MinifyResult(35-42)
packages/action/src/reporters/summary.ts (3)
examples/server.js (1)
result(27-27)packages/action/src/types.ts (2)
MinifyResult(35-42)BenchmarkResult(54-61)packages/utils/src/index.ts (1)
prettyBytes(48-48)
🪛 LanguageTool
AGENTS.md
[uncategorized] ~298-~298: The official name of this software platform is spelled with a capital “H”.
Context: ...ry includes a reusable GitHub Action at .github/actions/node-minify/ for CI/CD minific...
(GITHUB)
[uncategorized] ~323-~323: The official name of this software platform is spelled with a capital “H”.
Context: ....js) | ## CI/CD Workflows Located in .github/workflows/`: | Workflow | Trigger | Pu...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
316-316: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
🔇 Additional comments (5)
packages/action/src/checks.ts (1)
1-22: LGTM!The threshold checking logic is well-implemented with clear error messages and proper type safety using TypeScript's
Pickutility type. The function correctly handles both threshold scenarios (size increase and minimum reduction) with informative messages.packages/action/src/reporters/annotations.ts (1)
1-39: LGTM!The annotation logic is well-structured with appropriate severity levels (error, warning, notice) based on compression ratios. The threshold values (5% and 20%) are reasonable, and all annotations include helpful file context. The progressive severity checks ensure only one annotation per file, avoiding noise.
.github/workflows/release-action.yml (3)
3-12: Workflow triggers look correct; consider whetherrelease.prereleasedshould also be supported.
Currentrelease: types: [published]is fine if you only want fully published releases.
67-75: Verify the zipped artifact layout matchesaction.yml’sruns.main.
You’re copyingpackages/action/distinto the artifact asdist/at the root; this only works if rootaction.ymlpoints todist/index.js(notpackages/action/dist/index.js).
76-80: Release upload step looks good forreleaseevents.
Conditioning upload ongithub.event_name == 'release'is the right shape.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @srod. * #2760 (comment) The following files were modified: * `packages/action/src/checks.ts` * `packages/action/src/index.ts` * `packages/action/src/inputs.ts` * `packages/action/src/minify.ts` * `packages/action/src/outputs.ts` * `packages/action/src/reporters/annotations.ts` * `packages/action/src/reporters/comment.ts` * `packages/action/src/reporters/summary.ts` * `packages/benchmark/src/runner.ts` * `packages/utils/src/getFilesizeGzippedInBytes.ts` * `scripts/check-published.ts`
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/utils/__tests__/utils.test.ts:
- Around line 518-529: Update the tests for getFilesizeGzippedRaw to assert that
errors are wrapped in FileOperationError and add a directory-path case: replace
the generic rejects.toThrow() with an assertion that the promise rejects with an
instance of FileOperationError (referencing the FileOperationError class) when
called with a non-existent file name, and add another test that calls
getFilesizeGzippedRaw with a directory path (e.g., fixtureDir) and expects a
rejection with FileOperationError as well.
In @scripts/check-published.ts:
- Around line 81-84: The top-level promise rejection handler for main currently
logs the error without type-checking; replace the terse .catch handler with a
try/catch around awaiting main (or inside the .catch body) that verifies the
thrown value is an Error using if (err instanceof Error) and logs err.message
and err.stack (or otherwise logs a safe string when not an Error) before calling
process.exit(1); target the main() invocation and its existing catch block to
implement this pattern.
🧹 Nitpick comments (6)
AGENTS.md (3)
149-156: Document the newgetFilesizeGzippedRawutility function.The PR commit messages indicate a new utility
getFilesizeGzippedRawwas added, but it's not documented in the "File Operations" section of Common Patterns. If this is part of the public API from@node-minify/utils, it should be included alongsidegetFilesizeInBytesfor discoverability.📚 Proposed documentation addition
### File Operations (from @node-minify/utils) ```ts -import { readFile, writeFile, getFilesizeInBytes } from "@node-minify/utils"; +import { readFile, writeFile, getFilesizeInBytes, getFilesizeGzippedRaw } from "@node-minify/utils"; const content = await readFile("src/app.js"); await writeFile({ file: "dist/app.min.js", content, index: 0 }); const size = await getFilesizeInBytes("dist/app.min.js"); +const gzipSize = await getFilesizeGzippedRaw("dist/app.min.js");
310-314: Document threshold enforcement feature in GitHub Action section.The PR objectives mention "threshold enforcement to fail on size regressions," but this feature is not documented in the "Key Behaviors" section. If implemented, add an entry describing how thresholds are configured and enforced.
315-320: Add blank lines around the File table for markdown compliance.Markdownlint flags that tables should be surrounded by blank lines. The table at line 315–317 may be missing whitespace.
🔧 Proposed formatting fix
### Files + | File | Purpose | |------|---------| | `action.yml` | Action definition, inputs/outputs | +scripts/check-published.ts (1)
53-53: Removeawaitafter fixingcheckPublished.Once
checkPublishedis no longerasync, remove theawaithere.♻️ Proposed fix
- const status = await checkPublished(pkg.name, pkg.version); + const status = checkPublished(pkg.name, pkg.version);packages/action/src/reporters/comment.ts (1)
29-58: Consider adding error handling for GitHub API calls.The API calls to
listComments,updateComment, andcreateCommentcan fail (e.g., rate limiting, network issues, insufficient permissions). Currently, any failure will propagate up and fail the entire action. Consider wrapping these in try/catch and logging a warning instead, so PR comment failures don't break the minification workflow.♻️ Proposed improvement
export async function postPRComment( result: MinifyResult, githubToken: string | undefined ): Promise<void> { if (!githubToken) { warning("No GitHub token provided, skipping PR comment"); return; } const prNumber = context.payload.pull_request?.number; if (!prNumber) { warning("Not a pull request, skipping PR comment"); return; } - const octokit = getOctokit(githubToken); - const { owner, repo } = context.repo; - - const body = generateCommentBody(result); - - const comments = await octokit.paginate(octokit.rest.issues.listComments, { - owner, - repo, - issue_number: prNumber, - }); - - const existingComment = comments.find((c) => c.body?.includes(COMMENT_TAG)); - - if (existingComment) { - await octokit.rest.issues.updateComment({ - owner, - repo, - comment_id: existingComment.id, - body, - }); - info(`Updated existing PR comment #${existingComment.id}`); - } else { - const { data: newComment } = await octokit.rest.issues.createComment({ - owner, - repo, - issue_number: prNumber, - body, - }); - info(`Created new PR comment #${newComment.id}`); - } + try { + const octokit = getOctokit(githubToken); + const { owner, repo } = context.repo; + + const body = generateCommentBody(result); + + const comments = await octokit.paginate(octokit.rest.issues.listComments, { + owner, + repo, + issue_number: prNumber, + }); + + const existingComment = comments.find((c) => c.body?.includes(COMMENT_TAG)); + + if (existingComment) { + await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: existingComment.id, + body, + }); + info(`Updated existing PR comment #${existingComment.id}`); + } else { + const { data: newComment } = await octokit.rest.issues.createComment({ + owner, + repo, + issue_number: prNumber, + body, + }); + info(`Created new PR comment #${newComment.id}`); + } + } catch (error) { + if (error instanceof Error) { + warning(`Failed to post PR comment: ${error.message}`); + } else { + warning("Failed to post PR comment: Unknown error"); + } + } }packages/action/src/index.ts (1)
59-59: Consider using an IIFE or top-level await pattern.Calling
run()without handling the returned Promise means any unhandled rejection won't be caught at the top level. WhilesetFailedis called inside the try/catch, Node.js may still emit unhandled rejection warnings in edge cases.♻️ Optional: Handle the promise at the top level
-run(); +run().catch((error) => { + if (error instanceof Error) { + setFailed(error.message); + } else { + setFailed("An unknown error occurred"); + } +});
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.changeset/fluffy-eagles-sing.mdis excluded by!.changeset/**
📒 Files selected for processing (11)
.github/workflows/release-action.ymlAGENTS.mdpackages/action/README.mdpackages/action/__tests__/action.test.tspackages/action/src/checks.tspackages/action/src/index.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/comment.tspackages/action/src/reporters/summary.tspackages/utils/__tests__/utils.test.tsscripts/check-published.ts
✅ Files skipped from review due to trivial changes (1)
- packages/action/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/action/src/reporters/annotations.ts
- .github/workflows/release-action.yml
- packages/action/tests/action.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/checks.tspackages/action/src/reporters/summary.tspackages/utils/__tests__/utils.test.tspackages/action/src/reporters/comment.tsscripts/check-published.tspackages/action/src/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/checks.tspackages/action/src/reporters/summary.tspackages/utils/__tests__/utils.test.tspackages/action/src/reporters/comment.tsscripts/check-published.tspackages/action/src/index.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/utils/__tests__/utils.test.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/action/src/index.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/src/reporters/summary.tsAGENTS.mdpackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/action/src/reporters/summary.tspackages/utils/__tests__/utils.test.tsAGENTS.mdpackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/utils/__tests__/utils.test.tsAGENTS.mdpackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/utils/__tests__/utils.test.tsAGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Use Bun as the package manager and runtime for this repository (Node.js >=20.0.0, Bun 1.3.5+)
Applied to files:
AGENTS.mdscripts/check-published.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: All packages follow standard structure: `src/index.ts`, `__tests__/`, `package.json`, `tsconfig.json`, built with `tsdown`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
AGENTS.mdpackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `node:` prefix for Node.js built-in imports (e.g., `node:child_process`)
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Include `.ts` file extension in local imports (e.g., `./setup.ts`)
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx} : Use `type` keyword for type-only imports in TypeScript
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
AGENTS.md
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `try/catch` with `if (err instanceof Error)` checks for error handling
Applied to files:
scripts/check-published.ts
🧬 Code graph analysis (4)
packages/action/src/checks.ts (1)
packages/action/src/types.ts (1)
ActionInputs(7-23)
packages/action/src/reporters/summary.ts (2)
packages/action/src/types.ts (2)
MinifyResult(35-42)BenchmarkResult(54-61)packages/utils/src/index.ts (1)
prettyBytes(48-48)
packages/utils/__tests__/utils.test.ts (2)
packages/utils/src/index.ts (1)
getFilesizeGzippedRaw(41-41)packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(76-86)
packages/action/src/reporters/comment.ts (2)
packages/action/src/types.ts (1)
MinifyResult(35-42)packages/utils/src/index.ts (1)
prettyBytes(48-48)
🪛 LanguageTool
AGENTS.md
[uncategorized] ~298-~298: The official name of this software platform is spelled with a capital “H”.
Context: ...ry includes a reusable GitHub Action at .github/actions/node-minify/ for CI/CD minific...
(GITHUB)
[uncategorized] ~323-~323: The official name of this software platform is spelled with a capital “H”.
Context: ....js) | ## CI/CD Workflows Located in .github/workflows/`: | Workflow | Trigger | Pu...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
316-316: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
🔇 Additional comments (11)
AGENTS.md (1)
1-341: Excellent comprehensive update to guidance documentation.The AGENTS.md file has been substantially enriched with new sections covering the GitHub Action, CI/CD workflows, documentation site, anti-patterns, and CLI usage. The structure is clear, examples are practical, and all major learnings from the codebase (compressor resolution, file utilities, deprecation patterns, monorepo architecture) are well documented. The additions align well with the PR objectives and will serve future maintainers and AI assistants effectively.
scripts/check-published.ts (1)
16-39: Remove unnecessaryasyncand follow error handling guidelines.The function is marked
asyncbut doesn't useawait, making it unnecessarily async. Additionally, the error handling doesn't follow the coding guidelines.Issues:
- Line 16: The function doesn't await anything, so
asyncis unnecessary.- Line 36: The
catchblock doesn't checkif (err instanceof Error)as required by the coding guidelines.- Line 35: Missing ES5 trailing comma in the multi-line return object.
- Lines 23, 34: Checking for the string
"undefined"is fragile—npm shouldn't literally return this string.As per coding guidelines, error handling must use
if (err instanceof Error)checks.♻️ Proposed fixes
-async function checkPublished(packageName: string, version: string) { +function checkPublished(packageName: string, version: string) { try { const latest = execSync(`npm view ${packageName} version --json`, { stdio: "pipe", }) .toString() .trim(); - if (latest === "" || latest === "undefined") + if (latest === "") return { exists: false, publishedVersion: false }; const specific = execSync( `npm view ${packageName}@${version} version --json`, { stdio: "pipe" } ) .toString() .trim(); return { exists: true, - publishedVersion: specific !== "" && specific !== "undefined", + publishedVersion: specific !== "", }; - } catch (_e) { + } catch (err) { + if (err instanceof Error) { + // npm view throws when package doesn't exist + } return { exists: false, publishedVersion: false }; } }Likely an incorrect or invalid review comment.
packages/action/src/reporters/comment.ts (2)
1-10: LGTM!Copyright header is present and imports are correctly structured with the
typekeyword for type-only imports and.tsextension for local imports.
61-89: LGTM!The comment body generation is well-structured with a clear markdown table and collapsible configuration section. The use of
prettyBytesfor formatting and the hidden comment tag for identifying existing comments is appropriate.packages/action/src/index.ts (2)
1-16: LGTM!Copyright header is present. Imports are well-organized with proper
.tsextensions for local imports.
17-57: LGTM!The action flow is well-structured with clear separation of concerns:
- Input parsing and validation
- Minification execution
- Output setting
- Conditional reporting (summary, PR comment, annotations)
- Threshold enforcement
- Proper error handling with
instanceof ErrorcheckMinor note: Line 33 checks
context.payload.pull_requestbefore callingpostPRComment, which also checksprNumberinternally. This is redundant but harmless and provides defense in depth.packages/action/src/checks.ts (1)
1-22: LGTM!Clean implementation with:
- Proper copyright header
- Type-only import with
typekeyword- Precise type narrowing with
Pick<ActionInputs, ...>- Clear, testable logic for threshold validation
- Appropriate return type (
string | null) for error messagingpackages/action/src/reporters/summary.ts (3)
1-10: LGTM!Copyright header present. Imports are correctly structured with
.tsextension andtypekeyword for type-only imports.
11-41: LGTM!The
generateSummaryfunction correctly handles optionalgzipSizefield and produces a well-formatted summary table with proper use of the GitHub Actions summary API.
43-93: LGTM with minor observation on badge priority.The benchmark summary handles both success and failure cases appropriately with proper null checks for optional fields.
Note: The badge assignment logic (lines 60-63) uses an else-if chain, meaning if a compressor is both
bestSpeedandbestCompressionbut notrecommended, only the ⚡ (speed) badge will display. This may be intentional, but if you want both badges shown in that edge case, consider concatenating them.packages/utils/__tests__/utils.test.ts (1)
19-41: ImportinggetFilesizeGzippedRawfrom the public API is correct.
Nice to see the test consuming../src/index.ts(public surface) rather than deep-importing internals.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/check-published.ts (2)
16-39: Fix async declaration, add trailing comma, and improve error handling.
- Remove
asynckeyword: The function uses only synchronousexecSynccalls and contains noawaitstatements. Declaring it asasyncis misleading.- Add ES5 trailing comma: Line 35 in the multi-line return object is missing a trailing comma.
- Log error details: Line 36 catches errors but discards all details. This could hide real network/npm issues, making debugging harder.
♻️ Proposed fixes
-async function checkPublished(packageName: string, version: string) { +function checkPublished(packageName: string, version: string) { try { const latest = execSync(`npm view ${packageName} version --json`, { stdio: "pipe", }) .toString() .trim(); if (latest === "" || latest === "undefined") return { exists: false, publishedVersion: false }; const specific = execSync( `npm view ${packageName}@${version} version --json`, { stdio: "pipe" } ) .toString() .trim(); return { exists: true, - publishedVersion: specific !== "" && specific !== "undefined" + publishedVersion: specific !== "" && specific !== "undefined", }; - } catch (_e) { + } catch (err) { + // Package not found on npm or network error return { exists: false, publishedVersion: false }; } }As per coding guidelines, use ES5 trailing commas in multi-line objects.
47-55: Use parallel execution and async file utilities.
- Parallel package checks: The loop sequentially awaits each package check. For multiple packages, this is unnecessarily slow. Per coding guidelines, use
Promise.allfor parallel operations.- File utility: Line 49 uses
readFileSyncdirectly. Per coding guidelines, use the asyncreadFileutility from@node-minify/utils.⚡ Proposed refactor for parallel execution
First, update imports:
import { execSync } from "node:child_process"; -import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { existsSync, readdirSync } from "node:fs"; import { join } from "node:path"; +import { readFile } from "@node-minify/utils";Then refactor the loop to use parallel execution:
console.log("Checking packages..."); - for (const dir of dirs) { + const results = await Promise.all( + dirs.map(async (dir) => { - const pkgPath = join(PACKAGES_DIR, dir, "package.json"); + const pkgPath = join(PACKAGES_DIR, dir, "package.json"); - const pkg = JSON.parse(readFileSync(pkgPath, "utf-8")); + const content = await readFile(pkgPath, "utf-8"); + const pkg = JSON.parse(content); - if (pkg.private) continue; + if (pkg.private) return null; - const status = await checkPublished(pkg.name, pkg.version); + const status = checkPublished(pkg.name, pkg.version); - results.push({ name: pkg.name, version: pkg.version, ...status }); + return { name: pkg.name, version: pkg.version, ...status }; - } + }) + ).then((results) => results.filter((r) => r !== null));As per coding guidelines, use
Promise.allfor parallel operations and file utilities from@node-minify/utils. Based on learnings, use file operation utilities forreadFile.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/utils/__tests__/utils.test.tsscripts/check-published.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/utils/__tests__/utils.test.tsscripts/check-published.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/utils/__tests__/utils.test.tsscripts/check-published.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/utils/__tests__/utils.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/utils/__tests__/utils.test.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/utils/__tests__/utils.test.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `try/catch` with `if (err instanceof Error)` checks for error handling
Applied to files:
scripts/check-published.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Use Bun as the package manager and runtime for this repository (Node.js >=20.0.0, Bun 1.3.5+)
Applied to files:
scripts/check-published.ts
🧬 Code graph analysis (1)
packages/utils/__tests__/utils.test.ts (3)
packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(76-86)packages/utils/src/index.ts (1)
getFilesizeGzippedRaw(41-41)packages/utils/src/error.ts (1)
FileOperationError(11-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
🔇 Additional comments (3)
packages/utils/__tests__/utils.test.ts (2)
28-28: LGTM!The import of
getFilesizeGzippedRawis correctly added and follows the existing pattern.
518-536: Excellent test coverage for the new function!The test suite for
getFilesizeGzippedRawis well-structured and comprehensive. It appropriately tests the happy path by verifying the return type and positive value, and explicitly checks forFileOperationErrorin error cases, which is more precise than some existing tests in the file.scripts/check-published.ts (1)
81-89: LGTM!Error handling correctly follows the coding guidelines with
instanceof Errorchecks, appropriate logging, and error exit code.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="scripts/check-published.ts">
<violation number="1" location="scripts/check-published.ts:30">
P2: Regex for `packageName` validation rejects valid npm package names containing dots (e.g., `socket.io`). The pattern `/^[\w@/-]+$/` should include `.` to allow dots: `/^[\w@/.-]+$/`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/action/src/minify.ts:
- Around line 39-91: Wrap the body of runMinification in a try/catch to catch
errors from getFileSize, resolveCompressor, minify, and getFilesizeGzippedRaw;
in the catch, check if (err instanceof Error) and throw a new Error with
contextual information (e.g., include inputs.input, inputs.output and the
compressor label if available) or rethrow the original Error when not an Error
instance, so callers get meaningful messages; ensure you preserve the original
stack by including err.message or using Error's constructor and don't change the
function's returned MinifyResult shape except when propagating errors.
🧹 Nitpick comments (1)
packages/action/src/index.ts (1)
66-66: Add top-level error handling for the run() invocation.The top-level
run()call at line 66 is not wrapped in error handling. While therun()function has internal try-catch (lines 57-63), any errors thrown during the catch block itself (e.g., fromsetFailed) or synchronous errors before entering the try block would result in unhandled promise rejections.Recommended fix
-run(); +run().catch((error) => { + console.error("Fatal error:", error); + process.exit(1); +});
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/release-action.ymlpackages/action/src/checks.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/src/minify.tspackages/action/src/outputs.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/comment.tspackages/action/src/reporters/summary.tspackages/benchmark/src/runner.tspackages/utils/src/getFilesizeGzippedInBytes.tsscripts/check-published.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/action/src/outputs.ts
- packages/action/src/reporters/comment.ts
- packages/benchmark/src/runner.ts
- packages/action/src/inputs.ts
- packages/action/src/reporters/annotations.ts
- packages/action/src/checks.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/utils/src/getFilesizeGzippedInBytes.tspackages/action/src/minify.tspackages/action/src/reporters/summary.tspackages/action/src/index.tsscripts/check-published.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/utils/src/getFilesizeGzippedInBytes.tspackages/action/src/minify.tspackages/action/src/reporters/summary.tspackages/action/src/index.tsscripts/check-published.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/action/src/index.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/utils/src/getFilesizeGzippedInBytes.tspackages/action/src/minify.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/utils/src/getFilesizeGzippedInBytes.tspackages/action/src/minify.tspackages/action/src/reporters/summary.tspackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/src/minify.tspackages/action/src/reporters/summary.tspackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
packages/action/src/minify.tspackages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/action/src/minify.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `try/catch` with `if (err instanceof Error)` checks for error handling
Applied to files:
scripts/check-published.ts
🧬 Code graph analysis (2)
packages/utils/src/getFilesizeGzippedInBytes.ts (3)
packages/utils/src/error.ts (1)
FileOperationError(11-25)packages/utils/src/index.ts (3)
isValidFile(46-46)getFilesizeGzippedInBytes(40-40)getFilesizeGzippedRaw(41-41)packages/utils/src/isValidFile.ts (1)
isValidFile(21-27)
packages/action/src/minify.ts (4)
packages/action/src/types.ts (3)
ActionInputs(7-23)MinifyResult(35-42)FileResult(25-33)packages/utils/src/index.ts (2)
resolveCompressor(52-52)getFilesizeGzippedRaw(41-41)packages/core/src/index.ts (1)
minify(21-27)packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(75-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Greptile Review
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
🔇 Additional comments (7)
scripts/check-published.ts (1)
1-127: LGTM! Good security practices and error handling.The script demonstrates solid security practices:
- Input validation at line 30 prevents command injection
- Uses
execFileSyncinstead ofexecfor safer command execution- Proper error handling with
instanceof Errorchecks per coding guidelinesThe relaxed regex allowing dots in package names aligns with the commit message objective.
packages/action/src/reporters/summary.ts (1)
1-108: LGTM! Clean implementation with proper null handling.The summary reporters are well-structured with:
- Proper copyright headers per coding guidelines
- Correct use of
prettyBytesfrom@node-minify/utils- Good null/undefined handling for optional fields (lines 25, 82-85)
- Clear badge logic for benchmark results (lines 71-78)
- Appropriate fallbacks with "-" for missing data
The
!= nullcomparisons at lines 82-85 correctly check for bothnullandundefined, which is appropriate for optional TypeScript fields..github/workflows/release-action.yml (1)
43-70: The current implementation is secure; no additional sanitization is needed.The regex validation
^v[0-9]+(\.[0-9]+)*$at lines 56-59 is effective sanitization. It restricts input to onlyvfollowed by digits and dots, preventing any shell metacharacters (;,|,`,$, etc.) from passing through. This validation occurs immediately after the version is assigned and before any dangerous operations (git commands, file creation), which is the correct order. The outputs are subsequently quoted when used in shell commands (lines 79-80) and safely templated in filename contexts (lines 89, 95). No additional sanitization steps before the regex check are necessary.packages/action/src/minify.ts (4)
1-11: LGTM!The copyright header and imports follow all coding guidelines correctly:
node:prefix for built-ins,typekeyword for type-only imports,.tsextension for local imports, and proper use ofresolveCompressorfrom utils.
50-58: Clean conditional option passing.The use of conditional spreads for
typeandoptionsis well-implemented, keeping the minify call clean while only including properties when needed.
64-67: Reduction calculation handles edge cases correctly.The zero-division guard for
originalSize > 0prevents NaN results for empty files.
19-22: ThegetFileSizefunction is correctly implemented for its use case and should not be replaced.The
getFilesizeInBytesutility from@node-minify/utilsis a synchronous function that returns a formatted string (e.g., "45.2 kB"), whereas this function is asynchronous and returns a raw numeric byte count. These serve different purposes—one is for display, the other for size calculations (line 65-67). Direct substitution would break the code. Since no async raw-byte-count utility is exported from utils, the current implementation is appropriate.Likely an incorrect or invalid review comment.
| export async function runMinification( | ||
| inputs: ActionInputs | ||
| ): Promise<MinifyResult> { | ||
| const inputPath = resolve(inputs.workingDirectory, inputs.input); | ||
| const outputPath = resolve(inputs.workingDirectory, inputs.output); | ||
|
|
||
| const originalSize = await getFileSize(inputPath); | ||
| const { compressor, label } = await resolveCompressor(inputs.compressor); | ||
|
|
||
| const startTime = performance.now(); | ||
|
|
||
| await minify({ | ||
| compressor, | ||
| input: inputPath, | ||
| output: outputPath, | ||
| ...(inputs.type && { type: inputs.type }), | ||
| ...(Object.keys(inputs.options).length > 0 && { | ||
| options: inputs.options, | ||
| }), | ||
| }); | ||
|
|
||
| const endTime = performance.now(); | ||
| const timeMs = Math.round(endTime - startTime); | ||
|
|
||
| const minifiedSize = await getFileSize(outputPath); | ||
| const reduction = | ||
| originalSize > 0 | ||
| ? ((originalSize - minifiedSize) / originalSize) * 100 | ||
| : 0; | ||
|
|
||
| let gzipSize: number | undefined; | ||
| if (inputs.includeGzip) { | ||
| gzipSize = await getFilesizeGzippedRaw(outputPath); | ||
| } | ||
|
|
||
| const fileResult: FileResult = { | ||
| file: inputs.input, | ||
| originalSize, | ||
| minifiedSize, | ||
| reduction, | ||
| gzipSize, | ||
| timeMs, | ||
| }; | ||
|
|
||
| return { | ||
| files: [fileResult], | ||
| compressor: label, | ||
| totalOriginalSize: originalSize, | ||
| totalMinifiedSize: minifiedSize, | ||
| totalReduction: reduction, | ||
| totalTimeMs: timeMs, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add error handling for robustness.
The function performs multiple operations that can fail (file access, compressor resolution, minification), but lacks try/catch blocks. Per coding guidelines, use try/catch with if (err instanceof Error) checks to provide meaningful error context.
🛠️ Suggested error handling pattern
export async function runMinification(
inputs: ActionInputs
): Promise<MinifyResult> {
- const inputPath = resolve(inputs.workingDirectory, inputs.input);
- const outputPath = resolve(inputs.workingDirectory, inputs.output);
-
- const originalSize = await getFileSize(inputPath);
- const { compressor, label } = await resolveCompressor(inputs.compressor);
+ try {
+ const inputPath = resolve(inputs.workingDirectory, inputs.input);
+ const outputPath = resolve(inputs.workingDirectory, inputs.output);
+
+ const originalSize = await getFileSize(inputPath);
+ const { compressor, label } = await resolveCompressor(inputs.compressor);
- const startTime = performance.now();
+ const startTime = performance.now();
- await minify({
- compressor,
- input: inputPath,
- output: outputPath,
- ...(inputs.type && { type: inputs.type }),
- ...(Object.keys(inputs.options).length > 0 && {
- options: inputs.options,
- }),
- });
+ await minify({
+ compressor,
+ input: inputPath,
+ output: outputPath,
+ ...(inputs.type && { type: inputs.type }),
+ ...(Object.keys(inputs.options).length > 0 && {
+ options: inputs.options,
+ }),
+ });
- // ... rest of the function
+ // ... rest of the function
- return {
- files: [fileResult],
- compressor: label,
- totalOriginalSize: originalSize,
- totalMinifiedSize: minifiedSize,
- totalReduction: reduction,
- totalTimeMs: timeMs,
- };
+ return {
+ files: [fileResult],
+ compressor: label,
+ totalOriginalSize: originalSize,
+ totalMinifiedSize: minifiedSize,
+ totalReduction: reduction,
+ totalTimeMs: timeMs,
+ };
+ } catch (error) {
+ if (error instanceof Error) {
+ throw new Error(`Minification failed for ${inputs.input}: ${error.message}`);
+ }
+ throw error;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function runMinification( | |
| inputs: ActionInputs | |
| ): Promise<MinifyResult> { | |
| const inputPath = resolve(inputs.workingDirectory, inputs.input); | |
| const outputPath = resolve(inputs.workingDirectory, inputs.output); | |
| const originalSize = await getFileSize(inputPath); | |
| const { compressor, label } = await resolveCompressor(inputs.compressor); | |
| const startTime = performance.now(); | |
| await minify({ | |
| compressor, | |
| input: inputPath, | |
| output: outputPath, | |
| ...(inputs.type && { type: inputs.type }), | |
| ...(Object.keys(inputs.options).length > 0 && { | |
| options: inputs.options, | |
| }), | |
| }); | |
| const endTime = performance.now(); | |
| const timeMs = Math.round(endTime - startTime); | |
| const minifiedSize = await getFileSize(outputPath); | |
| const reduction = | |
| originalSize > 0 | |
| ? ((originalSize - minifiedSize) / originalSize) * 100 | |
| : 0; | |
| let gzipSize: number | undefined; | |
| if (inputs.includeGzip) { | |
| gzipSize = await getFilesizeGzippedRaw(outputPath); | |
| } | |
| const fileResult: FileResult = { | |
| file: inputs.input, | |
| originalSize, | |
| minifiedSize, | |
| reduction, | |
| gzipSize, | |
| timeMs, | |
| }; | |
| return { | |
| files: [fileResult], | |
| compressor: label, | |
| totalOriginalSize: originalSize, | |
| totalMinifiedSize: minifiedSize, | |
| totalReduction: reduction, | |
| totalTimeMs: timeMs, | |
| }; | |
| } | |
| export async function runMinification( | |
| inputs: ActionInputs | |
| ): Promise<MinifyResult> { | |
| try { | |
| const inputPath = resolve(inputs.workingDirectory, inputs.input); | |
| const outputPath = resolve(inputs.workingDirectory, inputs.output); | |
| const originalSize = await getFileSize(inputPath); | |
| const { compressor, label } = await resolveCompressor(inputs.compressor); | |
| const startTime = performance.now(); | |
| await minify({ | |
| compressor, | |
| input: inputPath, | |
| output: outputPath, | |
| ...(inputs.type && { type: inputs.type }), | |
| ...(Object.keys(inputs.options).length > 0 && { | |
| options: inputs.options, | |
| }), | |
| }); | |
| const endTime = performance.now(); | |
| const timeMs = Math.round(endTime - startTime); | |
| const minifiedSize = await getFileSize(outputPath); | |
| const reduction = | |
| originalSize > 0 | |
| ? ((originalSize - minifiedSize) / originalSize) * 100 | |
| : 0; | |
| let gzipSize: number | undefined; | |
| if (inputs.includeGzip) { | |
| gzipSize = await getFilesizeGzippedRaw(outputPath); | |
| } | |
| const fileResult: FileResult = { | |
| file: inputs.input, | |
| originalSize, | |
| minifiedSize, | |
| reduction, | |
| gzipSize, | |
| timeMs, | |
| }; | |
| return { | |
| files: [fileResult], | |
| compressor: label, | |
| totalOriginalSize: originalSize, | |
| totalMinifiedSize: minifiedSize, | |
| totalReduction: reduction, | |
| totalTimeMs: timeMs, | |
| }; | |
| } catch (error) { | |
| if (error instanceof Error) { | |
| throw new Error(`Minification failed for ${inputs.input}: ${error.message}`); | |
| } | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @packages/action/src/minify.ts around lines 39 - 91, Wrap the body of
runMinification in a try/catch to catch errors from getFileSize,
resolveCompressor, minify, and getFilesizeGzippedRaw; in the catch, check if
(err instanceof Error) and throw a new Error with contextual information (e.g.,
include inputs.input, inputs.output and the compressor label if available) or
rethrow the original Error when not an Error instance, so callers get meaningful
messages; ensure you preserve the original stack by including err.message or
using Error's constructor and don't change the function's returned MinifyResult
shape except when propagating errors.
| async function getGzipSize(file: string): Promise<number> { | ||
| if (!existsSync(file)) { | ||
| throw new FileOperationError( | ||
| "access", | ||
| file, | ||
| new Error("File does not exist") | ||
| ); | ||
| } | ||
|
|
||
| if (!isValidFile(file)) { | ||
| throw new FileOperationError( | ||
| "access", | ||
| file, | ||
| new Error("Path is not a valid file") | ||
| ); | ||
| } | ||
| if (!isValidFile(file)) { | ||
| throw new FileOperationError( | ||
| "access", | ||
| file, | ||
| new Error("Path is not a valid file") | ||
| ); | ||
| } | ||
|
|
||
| const { gzipSizeStream } = await import("gzip-size"); | ||
| const source = createReadStream(file); | ||
| const { gzipSizeStream } = await import("gzip-size"); | ||
| const source = createReadStream(file); | ||
|
|
||
| const size = await new Promise<number>((resolve, reject) => { | ||
| source | ||
| .pipe(gzipSizeStream()) | ||
| .on("gzip-size", resolve) | ||
| .on("error", reject); | ||
| }); | ||
| return new Promise<number>((resolve, reject) => { | ||
| source | ||
| .pipe(gzipSizeStream()) | ||
| .on("gzip-size", resolve) | ||
| .on("error", reject); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: getGzipSize throws FileOperationError but callers rewrap it.
The getGzipSize function throws FileOperationError at lines 22-26 and 30-34 for validation failures. However, both public callers (getFilesizeGzippedInBytes at lines 59-63 and getFilesizeGzippedRaw at lines 79-83) catch and rewrap all errors in a new FileOperationError, creating nested error chains.
This means a validation failure will be wrapped twice: first by getGzipSize, then again by the caller.
Suggested fix: Have getGzipSize throw FileOperationError consistently
Update getGzipSize to wrap all errors (including stream errors) in FileOperationError, then remove the error wrapping from the public functions:
async function getGzipSize(file: string): Promise<number> {
+ try {
if (!existsSync(file)) {
throw new FileOperationError(
"access",
file,
new Error("File does not exist")
);
}
if (!isValidFile(file)) {
throw new FileOperationError(
"access",
file,
new Error("Path is not a valid file")
);
}
const { gzipSizeStream } = await import("gzip-size");
const source = createReadStream(file);
return new Promise<number>((resolve, reject) => {
source
.pipe(gzipSizeStream())
.on("gzip-size", resolve)
.on("error", reject);
});
+ } catch (error) {
+ if (error instanceof FileOperationError) {
+ throw error;
+ }
+ throw new FileOperationError(
+ "get gzipped size of",
+ file,
+ error as Error
+ );
+ }
}Then simplify the public functions:
export async function getFilesizeGzippedInBytes(file: string): Promise<string> {
- try {
- const size = await getGzipSize(file);
- return prettyBytes(size);
- } catch (error) {
- throw new FileOperationError(
- "get gzipped size of",
- file,
- error as Error
- );
- }
+ const size = await getGzipSize(file);
+ return prettyBytes(size);
}
export async function getFilesizeGzippedRaw(file: string): Promise<number> {
- try {
- return await getGzipSize(file);
- } catch (error) {
- throw new FileOperationError(
- "get gzipped size of",
- file,
- error as Error
- );
- }
+ return await getGzipSize(file);
}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces a new GitHub Action package at packages/action to replace the legacy composite action, enabling marketplace publication with uses: srod/node-minify@v1.
Core Implementation
- New Node.js 20 action with bundled dependencies via Bun
- Main entrypoint at
packages/action/src/index.tsorchestrates minification workflow - Comprehensive reporting: job summaries, PR comments, and file annotations
- Threshold enforcement with
fail-on-increaseandmin-reductiongates - New
getFilesizeGzippedRawutility function for raw gzip sizes
Critical Issues Found
-
Action will not work: The root
action.ymlpoints topackages/action/dist/index.js, but this directory is gitignored and never committed. The release workflow builds dist but only creates a tag without committing the built files, so users will get "file not found" errors. -
Benchmark feature incomplete: Inputs, outputs, and documentation advertise benchmark comparison mode, but it's never implemented in
index.ts. Functions likegenerateBenchmarkSummaryexist but are never called.
Non-blocking Observations
- The deprecated composite action properly warns users to migrate
- Test workflow correctly builds and tests the action with multiple compressors
- Utility refactoring (getFilesizeGzippedRaw) is clean and well-tested
Confidence Score: 1/5
- Not safe to merge - action will fail when users try to use it due to missing dist directory
- Critical blocker: the action cannot function because action.yml references packages/action/dist/index.js which doesn't exist in the repository (dist is gitignored). The release workflow builds dist but never commits it, so the v1 tag will point to non-functional code. Additionally, the advertised benchmark feature is incomplete.
- .github/workflows/release-action.yml (must commit dist or change strategy), packages/action/src/index.ts (implement or remove benchmark feature), action.yml (references non-existent dist)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| action.yml | 0/5 | New root action.yml pointing to packages/action/dist/index.js which doesn't exist in repository (dist is gitignored). Action will fail when used. |
| .github/workflows/release-action.yml | 1/5 | New release workflow builds dist but never commits it to repository, causing action.yml to reference non-existent files. Missing git add/commit step. |
| packages/action/src/index.ts | 2/5 | Main action entrypoint. Benchmark feature (inputs.benchmark) is parsed but never implemented - missing runBenchmark call and benchmark output generation. |
| packages/action/action.yml | 3/5 | Package-level action.yml defining inputs/outputs including benchmark features. References dist/index.js which must be built. |
| packages/action/src/inputs.ts | 4/5 | Input parsing and validation. Correctly parses benchmark inputs but they're never used in main workflow. |
| packages/action/src/outputs.ts | 4/5 | Output generation with setBenchmarkOutputs function that's never called from main workflow. |
| packages/action/src/reporters/summary.ts | 4/5 | Summary reporters including generateBenchmarkSummary function that's never called from main workflow. |
| packages/action/src/minify.ts | 5/5 | Minification logic that resolves compressor and runs minify operation. Works correctly for single-file minification. |
| packages/action/src/checks.ts | 5/5 | Threshold checking logic for fail-on-increase and min-reduction. Clean implementation with correct validation. |
| packages/utils/src/getFilesizeGzippedInBytes.ts | 5/5 | Adds getFilesizeGzippedRaw function for raw byte gzip size. Refactored cleanly with shared getGzipSize helper. |
| packages/utils/src/index.ts | 5/5 | Exports new getFilesizeGzippedRaw function. Properly added to exports list. |
| .github/workflows/test-action.yml | 4/5 | Updated test workflow builds action and tests with terser, esbuild, lightningcss. Properly uploads/downloads dist artifact between jobs. |
| docs/src/content/docs/github-action.md | 3/5 | Comprehensive docs for GitHub Action including benchmark examples. Documents features that aren't fully implemented yet. |
| packages/action/package.json | 5/5 | Package configuration with Bun build script. Properly configured as private package with correct dependencies. |
| .github/actions/node-minify/action.yml | 5/5 | Old composite action now deprecated with warning messages. Properly guides users to migrate to new action. |
Sequence Diagram
sequenceDiagram
participant User as GitHub Workflow
participant Action as action.yml (root)
participant Main as packages/action/src/index.ts
participant Inputs as src/inputs.ts
participant Minify as src/minify.ts
participant Core as @node-minify/core
participant Reporters as src/reporters/*
participant Outputs as src/outputs.ts
User->>Action: uses: srod/node-minify@v1
Action->>Main: Execute packages/action/dist/index.js
Note over Action,Main: ❌ BROKEN: dist/index.js doesn't exist in repo
Main->>Inputs: parseInputs()
Inputs-->>Main: ActionInputs (including benchmark flag)
Main->>Inputs: validateJavaCompressor()
Main->>Minify: runMinification(inputs)
Minify->>Core: minify({ compressor, input, output })
Core-->>Minify: minified file written
Minify-->>Main: MinifyResult
Main->>Outputs: setMinifyOutputs(result)
Outputs-->>User: Set original-size, minified-size, etc.
alt reportSummary enabled
Main->>Reporters: generateSummary(result)
Reporters-->>User: Add job summary
end
alt reportPRComment enabled
Main->>Reporters: postPRComment(result, token)
Reporters-->>User: Create/update PR comment
end
alt reportAnnotations enabled
Main->>Reporters: addAnnotations(result)
Reporters-->>User: Add file annotations
end
Note over Main,Reporters: ❌ MISSING: benchmark path never executed
Note over Main: inputs.benchmark is parsed but ignored
Note over Reporters: generateBenchmarkSummary exists but never called
Note over Outputs: setBenchmarkOutputs exists but never called
Main->>Main: checkThresholds(result.totalReduction, inputs)
alt threshold violated
Main-->>User: setFailed(error)
else success
Main-->>User: Complete ✅
end
.github/workflows/release-action.yml
Outdated
| - name: Update major version tag | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| # Force update the major version tag (e.g., v1) | ||
| MAJOR="${{ steps.version.outputs.major }}" | ||
| git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}" | ||
| git push origin "$MAJOR" --force |
There was a problem hiding this comment.
[P0] The workflow builds packages/action/dist/ but never commits it to the repository. The v1 tag will point to code without the dist directory, causing the action to fail when users run uses: srod/node-minify@v1 because action.yml references packages/action/dist/index.js which doesn't exist.
You need to either: (1) commit the dist directory after building it, or (2) remove dist from .gitignore and commit it with the PR.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-action.yml
Line: 72:80
Comment:
[P0] The workflow builds `packages/action/dist/` but never commits it to the repository. The `v1` tag will point to code without the dist directory, causing the action to fail when users run `uses: srod/node-minify@v1` because `action.yml` references `packages/action/dist/index.js` which doesn't exist.
You need to either: (1) commit the dist directory after building it, or (2) remove dist from .gitignore and commit it with the PR.
How can I resolve this? If you propose a fix, please make it concise.| const result = await runMinification(inputs); | ||
|
|
||
| setMinifyOutputs(result); | ||
|
|
||
| if (inputs.reportSummary) { | ||
| await generateSummary(result); | ||
| } | ||
|
|
||
| if (inputs.reportPRComment && context.payload.pull_request) { | ||
| await postPRComment(result, inputs.githubToken); | ||
| } | ||
|
|
||
| if (inputs.reportAnnotations) { | ||
| addAnnotations(result); | ||
| } |
There was a problem hiding this comment.
[P1] The benchmark input is declared in action.yml and parsed in inputs.ts, but the benchmark functionality is never implemented. When users set benchmark: true, nothing happens - no benchmark runs, no benchmark summary is generated, and the benchmark-winner output is never set.
The generateBenchmarkSummary and setBenchmarkOutputs functions exist but are never called. Either implement the feature or remove it from the inputs/outputs/docs until ready.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/index.ts
Line: 32:46
Comment:
[P1] The `benchmark` input is declared in action.yml and parsed in inputs.ts, but the benchmark functionality is never implemented. When users set `benchmark: true`, nothing happens - no benchmark runs, no benchmark summary is generated, and the `benchmark-winner` output is never set.
The `generateBenchmarkSummary` and `setBenchmarkOutputs` functions exist but are never called. Either implement the feature or remove it from the inputs/outputs/docs until ready.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/release-action.yml:
- Around line 97-101: The workflow step "Upload release artifact" is gated by
the condition if: github.event_name == 'release', which prevents artifacts from
being uploaded when the workflow is manually dispatched; either remove that
condition so the step runs for manual_dispatch events as well (or adjust to if:
contains(['release','workflow_dispatch'], github.event_name)) or explicitly
document in the README/workflow comments that manual dispatches will not upload
artifacts so users are not surprised; update the step that uses
softprops/action-gh-release@v2 and its files: node-minify-action-${{
steps.version.outputs.version }}.zip accordingly.
- Around line 72-79: The "Commit built action dist" step currently suppresses
git commit failures with `|| echo "No changes to commit"`, which lets the
workflow continue and tag the wrong commit; change this so a missing commit
causes the job to fail explicitly by removing the suppression and instead
checking for staged changes or testing the git commit exit code and exiting
non‑zero if nothing was committed (e.g., run a pre-commit check like `git status
--porcelain` or evaluate the `git commit` return and call exit 1 on failure) for
the git commit command shown in the step.
- Around line 81-86: The release workflow risks race conditions when multiple
runs update the same major tag: add a top-level concurrency stanza (e.g.,
concurrency: release-${{ github.ref_name }}) so only one release run for a given
ref executes at a time, and modify the "Update major version tag" step to detect
and fail on unsuccessful pushes (check git push exit status and surface an
error) rather than silently forcing the tag; reference the existing step name
"Update major version tag" and the MAJOR variable/usage (git tag -fa "$MAJOR"
... git push origin "$MAJOR" --force) when making these changes.
🧹 Nitpick comments (1)
.github/workflows/release-action.yml (1)
26-41: Consider adding verification that the built action works before releasing.The workflow builds and releases the action without testing it. A quick verification step (e.g., running the action in a test mode or checking that required files exist in dist) could catch build issues before they reach users.
Example verification step
Add this step after "Build action" and before "Get version":
- name: Verify action build run: | # Check that the bundled action exists if [ ! -f packages/action/dist/index.js ]; then echo "Error: Built action bundle not found at packages/action/dist/index.js" exit 1 fi # Optionally: verify the bundle size is reasonable SIZE=$(stat -f%z packages/action/dist/index.js 2>/dev/null || stat -c%s packages/action/dist/index.js) if [ "$SIZE" -lt 1000 ]; then echo "Error: Built bundle suspiciously small ($SIZE bytes)" exit 1 fi echo "✓ Action bundle verified ($SIZE bytes)"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-action.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
🔇 Additional comments (2)
.github/workflows/release-action.yml (2)
21-24: LGTM! Checkout ref logic handles both trigger types correctly.The fallback chain
github.event.inputs.tag || (github.event.release.tag_name || github.ref)correctly prioritizes manual dispatch input over release tag over current ref.
43-70: LGTM! Version extraction and validation are robust.The regex validation
^v[0-9]+(\.[0-9]+)*$correctly enforces semver-style tags, and major version extraction using^v([0-9]+)is accurate.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/release-action.yml">
<violation number="1" location=".github/workflows/release-action.yml:81">
P2: Removing the `|| echo "No changes to commit"` fallback causes the workflow to fail if dist files are unchanged. This could happen if a release is re-run or the build output is identical. Consider keeping the fallback or using `git diff --cached --quiet || git commit ...` to only commit when there are actual changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/test-action.yml (2)
67-75: Consider simplifying compressor package installation.Installing packages to
/tmpand copying to$GITHUB_WORKSPACE/node_modules/is fragile. Ifnode_modulesalready has packages from the checkout (e.g., from a lockfile), thecp -rcould produce unexpected results due to file overwrites.♻️ Suggested simplification
- name: Install compressor packages run: | - mkdir -p /tmp/compressors && cd /tmp/compressors - npm init -y - npm install @node-minify/terser @node-minify/esbuild - - # Create node_modules and copy packages - mkdir -p $GITHUB_WORKSPACE/node_modules - cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ + npm install --no-save @node-minify/terser @node-minify/esbuildUsing
npm install --no-savedirectly in the workspace is simpler and avoids potential copy conflicts.
170-178: Same installation simplification applies here.The CSS compressor installation has the same fragility as the JS test. Consider using
npm install --no-save @node-minify/lightningcssdirectly.♻️ Suggested simplification
- name: Install compressor packages run: | - mkdir -p /tmp/compressors && cd /tmp/compressors - npm init -y - npm install @node-minify/lightningcss - - # Create node_modules and copy packages - mkdir -p $GITHUB_WORKSPACE/node_modules - cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ + npm install --no-save @node-minify/lightningcss
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/release-action.yml.github/workflows/test-action.ymlpackages/action/package.jsonpackages/action/src/benchmark.tspackages/action/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/action/src/index.ts
- packages/action/package.json
- .github/workflows/release-action.yml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/benchmark.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/benchmark.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/action/src/benchmark.ts.github/workflows/test-action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/action/src/benchmark.ts.github/workflows/test-action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
packages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
.github/workflows/test-action.yml
🪛 actionlint (1.7.10)
.github/workflows/test-action.yml
209-209: file "packages/action/dist/index.js" does not exist in "/home/jailuser/git". it is specified at "main" key in "runs" section in "node-minify" action
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
🔇 Additional comments (8)
.github/workflows/test-action.yml (6)
3-15: LGTM!The workflow trigger configuration correctly includes the new action package paths and relevant branches. Path filtering ensures efficient CI runs.
18-47: LGTM!The build-action job correctly builds dependencies in order, bundles the action, and uploads the artifact for downstream test jobs. The static analysis warning about
packages/action/dist/index.jsnot existing is expected since the file is generated during this build step.
94-127: LGTM!The terser minification test correctly verifies both file creation and positive reduction percentage. The
bc -lcomparison is appropriate for floating-point values.
129-150: LGTM!The esbuild test and comparison summary provide useful metrics. The step summary table format is clean and informative.
207-227: LGTM!The CSS minification test correctly verifies lightningcss output and reduction percentage. Direct variable interpolation in the shell script is acceptable for this test context.
229-235: LGTM!The summary job serves as an effective aggregator for the required checks, ensuring both JS and CSS tests pass before reporting success.
packages/action/src/benchmark.ts (2)
1-10: Nice start: header + imports align with repo conventions.
Usesnode:import prefix,typeimport, and.tslocal import extension; style looks consistent.
20-71: Type definitions guaranteeresult.filesandresult.summaryalways exist; no undefined guards needed.The
BenchmarkResultinterface explicitly marks bothfiles: FileResult[]andsummaryas required (non-optional) properties. TherunBenchmarkfunction inpackages/benchmark/src/runner.tsalways creates both properties before returning. Adding guards for undefined is unnecessary and masks the actual safe access pattern already in place.The current code correctly handles the only realistic edge case: when
filesis an empty array (no files matched the input pattern), theif (!fileResult)check returns a safe empty result.Consider adding try/catch around the
benchmark()call with error context if that function can throw, but the proposed null-coalescing operators are redundant given the type guarantees.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces a comprehensive GitHub Action package for node-minify with bundled dependencies, rich reporting features, and benchmark capabilities. The implementation is well-structured with proper separation of concerns across input parsing, minification, benchmarking, reporting, and output handling.
Key Strengths
- Clean Architecture: Well-organized code with clear separation between minification, benchmarking, reporting, and threshold checking
- Comprehensive Features: Supports job summaries, PR comments, file annotations, benchmark comparisons, and threshold enforcement
- Good Documentation: Detailed README and docs/github-action.md with examples for all major use cases
- Proper Error Handling: Try-catch blocks, input validation, and clear error messages throughout
- Type Safety: Strong TypeScript types with interfaces for all major data structures
Critical Issues Found
1. Release Workflow - Commit Not Pushed (Blocker)
The release workflow commits built dist files but never pushes the commit to the remote. It only pushes the tag, which means the dist files won't be available when users checkout the tag. This will cause the action to fail for all users.
2. Release Workflow - workflow_dispatch Artifact Upload (Blocker)
The artifact upload step runs for both release and workflow_dispatch events, but softprops/action-gh-release requires an existing release. For workflow_dispatch, no release exists yet, causing the step to fail.
3. Benchmark Gzip Data Loss (High Impact)
The benchmark integration filters out gzip data with a type check that converts strings to undefined. Since the benchmark package returns gzipSize as a formatted string, gzip information is never displayed in benchmark results even when includeGzip: true.
4. Missing Output Declarations (Medium Impact)
The code sets best-compression and best-speed outputs but these are not declared in action.yml files. GitHub Actions won't expose these outputs to users.
5. Silent Input Validation Failure (Medium Impact)
Invalid min-reduction input (like "abc") is silently converted to 0 instead of throwing an error, preventing users from knowing their threshold configuration is invalid.
Recommendations
- Fix the release workflow immediately - This is a deployment blocker that will prevent the action from working
- Fix benchmark gzip handling - Use
gzipBytesfield or remove the type filter - Add missing output declarations to both action.yml files
- Add validation for min-reduction to fail fast on invalid input
- Consider adding integration tests - Current tests only cover types and threshold logic
The core minification and reporting logic is solid, but the deployment workflow and benchmark integration need fixes before release.
Confidence Score: 2/5
- This PR has critical deployment issues that will prevent the action from working when released
- While the core action implementation is well-designed with good error handling and comprehensive features, the release workflow contains two blocking bugs: (1) the commit with dist files is never pushed to remote, and (2) workflow_dispatch artifact upload will fail. These issues will prevent successful deployment and break the action for all users. Additionally, benchmark gzip data is lost due to a type mismatch. The score reflects that this cannot safely merge until the release workflow is fixed.
- .github/workflows/release-action.yml requires immediate fixes (missing git push, invalid artifact upload condition), packages/action/src/benchmark.ts needs gzip handling correction, and both action.yml files need output declarations added
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| .github/workflows/release-action.yml | 1/5 | New release workflow with critical bugs: commit not pushed, workflow_dispatch artifact upload will fail |
| packages/action/src/index.ts | 4/5 | Main action orchestration logic with proper error handling and feature integration |
| packages/action/src/benchmark.ts | 2/5 | Benchmark integration with gzip data type mismatch causing data loss |
| packages/action/src/inputs.ts | 3/5 | Input parsing with silent failure on invalid min-reduction values |
| packages/action/src/outputs.ts | 3/5 | Sets outputs including undeclared best-compression and best-speed |
| action.yml | 4/5 | Root action metadata pointing to bundled action with complete input/output definitions |
| packages/action/action.yml | 4/5 | Package action metadata identical to root, missing best-compression/best-speed outputs |
Sequence Diagram
sequenceDiagram
participant User as GitHub Workflow
participant Action as index.ts
participant Inputs as inputs.ts
participant Minify as minify.ts
participant Benchmark as benchmark.ts
participant Outputs as outputs.ts
participant Reporters as reporters/*
participant Checks as checks.ts
User->>Action: Trigger action
Action->>Inputs: parseInputs()
Inputs-->>Action: ActionInputs
Action->>Inputs: validateJavaCompressor()
Action->>Minify: runMinification(inputs)
Minify->>Minify: getFileSize(input)
Minify->>Minify: resolveCompressor()
Minify->>Minify: minify()
Minify->>Minify: getFileSize(output)
Minify->>Minify: getFilesizeGzippedRaw() [optional]
Minify-->>Action: MinifyResult
Action->>Outputs: setMinifyOutputs(result)
Outputs-->>User: Set action outputs
alt reportSummary enabled
Action->>Reporters: generateSummary(result)
Reporters-->>User: Job summary
end
alt reportPRComment enabled
Action->>Reporters: postPRComment(result, token)
Reporters-->>User: PR comment
end
alt reportAnnotations enabled
Action->>Reporters: addAnnotations(result)
Reporters-->>User: File annotations
end
alt benchmark enabled
Action->>Benchmark: runBenchmark(inputs)
Benchmark->>Benchmark: benchmark multiple compressors
Benchmark-->>Action: BenchmarkResult
Action->>Outputs: setBenchmarkOutputs(result)
alt reportSummary enabled
Action->>Reporters: generateBenchmarkSummary(result)
Reporters-->>User: Benchmark summary
end
end
Action->>Checks: checkThresholds(reduction, inputs)
alt threshold violated
Checks-->>Action: Error message
Action->>User: setFailed()
else threshold met
Checks-->>Action: null
Action->>User: Success
end
| - name: Commit built action dist | ||
| run: | | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| # Force add dist (ignored by .gitignore) | ||
| git add -f packages/action/dist/ | ||
| git commit -m "chore: build action dist for ${{ steps.version.outputs.version }}" | ||
|
|
||
| - name: Update major version tag | ||
| run: | | ||
| # Force update the major version tag (e.g., v1) | ||
| MAJOR="${{ steps.version.outputs.major }}" | ||
| git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}" | ||
| if ! git push origin "$MAJOR" --force; then | ||
| echo "Error: Failed to push major version tag $MAJOR" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The commit with built dist files is never pushed to the remote repository. After committing on line 81, the workflow creates and pushes a tag on line 87-88, but the commit itself is never pushed. This means when users checkout the v1 tag, the dist files won't exist because the commit wasn't pushed to the repository.
| - name: Commit built action dist | |
| run: | | |
| git config user.name "github-actions[bot]" | |
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| # Force add dist (ignored by .gitignore) | |
| git add -f packages/action/dist/ | |
| git commit -m "chore: build action dist for ${{ steps.version.outputs.version }}" | |
| - name: Update major version tag | |
| run: | | |
| # Force update the major version tag (e.g., v1) | |
| MAJOR="${{ steps.version.outputs.major }}" | |
| git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}" | |
| if ! git push origin "$MAJOR" --force; then | |
| echo "Error: Failed to push major version tag $MAJOR" | |
| exit 1 | |
| fi | |
| - name: Commit built action dist | |
| run: | | |
| git config user.name "github-actions[bot]" | |
| git config user.email "github-actions[bot]@users.noreply.github.com" | |
| # Force add dist (ignored by .gitignore) | |
| git add -f packages/action/dist/ | |
| git commit -m "chore: build action dist for ${{ steps.version.outputs.version }}" | |
| # Push the commit before tagging | |
| git push origin HEAD:${{ github.ref_name }} | |
| - name: Update major version tag | |
| run: | | |
| # Force update the major version tag (e.g., v1) | |
| MAJOR="${{ steps.version.outputs.major }}" | |
| git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}" | |
| if ! git push origin "$MAJOR" --force; then | |
| echo "Error: Failed to push major version tag $MAJOR" | |
| exit 1 | |
| fi |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-action.yml
Line: 74:91
Comment:
The commit with built dist files is never pushed to the remote repository. After committing on line 81, the workflow creates and pushes a tag on line 87-88, but the commit itself is never pushed. This means when users checkout the v1 tag, the dist files won't exist because the commit wasn't pushed to the repository.
```suggestion
- name: Commit built action dist
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
# Force add dist (ignored by .gitignore)
git add -f packages/action/dist/
git commit -m "chore: build action dist for ${{ steps.version.outputs.version }}"
# Push the commit before tagging
git push origin HEAD:${{ github.ref_name }}
- name: Update major version tag
run: |
# Force update the major version tag (e.g., v1)
MAJOR="${{ steps.version.outputs.major }}"
git tag -fa "$MAJOR" -m "Update $MAJOR tag to ${{ steps.version.outputs.version }}"
if ! git push origin "$MAJOR" --force; then
echo "Error: Failed to push major version tag $MAJOR"
exit 1
fi
```
How can I resolve this? If you propose a fix, please make it concise.| - name: Upload release artifact | ||
| if: contains(fromJson('["release", "workflow_dispatch"]'), github.event_name) | ||
| uses: softprops/action-gh-release@v2 | ||
| with: | ||
| files: node-minify-action-${{ steps.version.outputs.version }}.zip |
There was a problem hiding this comment.
When triggered via workflow_dispatch, this step will fail because softprops/action-gh-release@v2 expects an existing release to attach files to. For workflow_dispatch events, no release exists yet (only the tag input is provided). The workflow should either skip this step for workflow_dispatch or create a release first.
| - name: Upload release artifact | |
| if: contains(fromJson('["release", "workflow_dispatch"]'), github.event_name) | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: node-minify-action-${{ steps.version.outputs.version }}.zip | |
| - name: Upload release artifact | |
| if: github.event_name == 'release' | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: node-minify-action-${{ steps.version.outputs.version }}.zip |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-action.yml
Line: 102:106
Comment:
When triggered via `workflow_dispatch`, this step will fail because `softprops/action-gh-release@v2` expects an existing release to attach files to. For `workflow_dispatch` events, no release exists yet (only the tag input is provided). The workflow should either skip this step for `workflow_dispatch` or create a release first.
```suggestion
- name: Upload release artifact
if: github.event_name == 'release'
uses: softprops/action-gh-release@v2
with:
files: node-minify-action-${{ steps.version.outputs.version }}.zip
```
How can I resolve this? If you propose a fix, please make it concise.
packages/action/src/benchmark.ts
Outdated
| success: r.success, | ||
| size: r.sizeBytes, | ||
| reduction: r.reductionPercent, | ||
| gzipSize: typeof r.gzipSize === "string" ? undefined : r.gzipSize, |
There was a problem hiding this comment.
The benchmark package returns gzipSize as a formatted string (e.g., "1.5 kB") from getFilesizeGzippedInBytes, but this code converts all string values to undefined. This means gzip data will never be included in benchmark results, even when includeGzip is true.
The benchmark package's CompressorMetrics interface has both gzipSize (string) and gzipBytes (number) fields, but only gzipSize is populated. You should either:
- Use
r.gzipBytesif available, or - Remove the type check and keep the string value
| gzipSize: typeof r.gzipSize === "string" ? undefined : r.gzipSize, | |
| gzipSize: r.gzipBytes, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/benchmark.ts
Line: 55:55
Comment:
The benchmark package returns `gzipSize` as a formatted string (e.g., "1.5 kB") from `getFilesizeGzippedInBytes`, but this code converts all string values to `undefined`. This means gzip data will never be included in benchmark results, even when `includeGzip` is true.
The benchmark package's `CompressorMetrics` interface has both `gzipSize` (string) and `gzipBytes` (number) fields, but only `gzipSize` is populated. You should either:
1. Use `r.gzipBytes` if available, or
2. Remove the type check and keep the string value
```suggestion
gzipSize: r.gzipBytes,
```
How can I resolve this? If you propose a fix, please make it concise.
packages/action/src/inputs.ts
Outdated
| benchmark: getBooleanInput("benchmark"), | ||
| benchmarkCompressors, | ||
| failOnIncrease: getBooleanInput("fail-on-increase"), | ||
| minReduction: Number.parseFloat(getInput("min-reduction")) || 0, |
There was a problem hiding this comment.
Invalid min-reduction input is silently ignored. If a user provides "abc" or other non-numeric input, Number.parseFloat("abc") returns NaN, and NaN || 0 evaluates to 0. The action should throw an error for invalid input so users know their threshold isn't being applied.
| minReduction: Number.parseFloat(getInput("min-reduction")) || 0, | |
| minReduction: (() => { | |
| const value = Number.parseFloat(getInput("min-reduction")) || 0; | |
| if (isNaN(value) && getInput("min-reduction")) { | |
| throw new Error(`Invalid 'min-reduction' input: must be a number between 0 and 100`); | |
| } | |
| return value; | |
| })(), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/inputs.ts
Line: 76:76
Comment:
Invalid `min-reduction` input is silently ignored. If a user provides "abc" or other non-numeric input, `Number.parseFloat("abc")` returns `NaN`, and `NaN || 0` evaluates to `0`. The action should throw an error for invalid input so users know their threshold isn't being applied.
```suggestion
minReduction: (() => {
const value = Number.parseFloat(getInput("min-reduction")) || 0;
if (isNaN(value) && getInput("min-reduction")) {
throw new Error(`Invalid 'min-reduction' input: must be a number between 0 and 100`);
}
return value;
})(),
```
How can I resolve this? If you propose a fix, please make it concise.| if (result.bestCompression) { | ||
| setOutput("best-compression", result.bestCompression); | ||
| } | ||
| if (result.bestSpeed) { | ||
| setOutput("best-speed", result.bestSpeed); |
There was a problem hiding this comment.
The outputs best-compression and best-speed are set here but are not declared in action.yml or packages/action/action.yml. GitHub Actions will not expose these outputs to users unless they are declared in the action metadata file.
Add these output declarations to both action.yml files:
best-compression:
description: "Compressor with best compression ratio (if benchmark run)"
best-speed:
description: "Fastest compressor (if benchmark run)"Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/outputs.ts
Line: 42:46
Comment:
The outputs `best-compression` and `best-speed` are set here but are not declared in `action.yml` or `packages/action/action.yml`. GitHub Actions will not expose these outputs to users unless they are declared in the action metadata file.
Add these output declarations to both action.yml files:
```yaml
best-compression:
description: "Compressor with best compression ratio (if benchmark run)"
best-speed:
description: "Fastest compressor (if benchmark run)"
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/action/src/inputs.ts">
<violation number="1" location="packages/action/src/inputs.ts:82">
P2: The error message claims 'expected 0-100' but no range validation is performed. Add range validation to match the stated expectation.</violation>
</file>
<file name=".github/workflows/release-action.yml">
<violation number="1" location=".github/workflows/release-action.yml:87">
P1: The push target `${{ github.ref_name }}` is incorrect for `workflow_dispatch` triggers. When manually triggered, `github.ref_name` is the branch (e.g., 'main'), not the input tag. This will push commits to the wrong ref. Use `${{ steps.version.outputs.version }}` instead to consistently push to the correct tag for both trigger types.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/release-action.yml:
- Around line 74-99: The workflow currently pushes the dist commit to origin
HEAD:${{ github.ref_name }} which creates/updates a branch named like the
release tag and leaves the release tag pointing at the pre-dist commit; instead,
after committing the built dist (the commit created by the git commit in the
"Commit built action dist" step that uses steps.version.outputs.version), create
or update the release tag named ${{ github.ref_name }} to point at HEAD (the new
dist commit) and push that tag to origin (force if needed), then proceed to
update/push the MAJOR tag (MAJOR="${{ steps.version.outputs.major }}") as in the
"Update major version tag" step; replace the git push origin HEAD:${{
github.ref_name }} with an explicit tag update/push for ${{ github.ref_name }}.
- Around line 100-113: The packaged artifact copies action.yml into
release-action but the dist files are placed at release-action/dist/, causing
the action.yml main path (main: "packages/action/dist/index.js") to be invalid;
fix by either updating action.yml's main to "dist/index.js" as part of the
packaging step (modify the workflow to rewrite action.yml before zipping) or
preserve the original path by copying the packages/action/ directory into the
archive instead of only its dist contents (adjust the mkdir/cp commands to keep
packages/action/dist together), ensuring the action.yml main and the artifact
layout match.
- Line 13: The concurrency group currently keys off github.ref_name (the branch)
which can cause manual dispatches from the same branch to collide; change the
concurrency key in the release workflow to use the release tag identifier
instead (use github.event.release.tag_name when available, falling back to
github.ref_name) so the group is keyed by tag (release-TAG) rather than branch;
update the concurrency declaration that currently references github.ref_name
accordingly.
In @packages/action/src/inputs.ts:
- Line 12: Remove the unused constant JAVA_COMPRESSORS from
packages/action/src/inputs.ts: delete the declaration (and any export it may
have) so it is not exported from the module, leaving validateJavaCompressor
intact; after removal, run tests/linters to ensure nothing else imports or
depends on JAVA_COMPRESSORS and update any export lists or documentation if it
was part of a public API contract.
🧹 Nitpick comments (4)
packages/action/src/benchmark.ts (1)
33-45: Consider documenting the single-file assumption.The code safely handles the case where
result.files[0]is undefined, but the single-file assumption is implicit. Since the action operates on a single input file (frominputs.input), this is correct, but a brief comment explaining why only the first file is used would improve clarity.📝 Optional documentation improvement
- // Convert benchmark package result to action's BenchmarkResult type - // The benchmark package returns results per-file, we take the first file + // Convert benchmark package result to action's BenchmarkResult type. + // The benchmark package returns results per-file. Since the action runs + // on a single input file, we take the first (and only expected) file result. const fileResult = result.files[0];packages/action/src/inputs.ts (1)
68-68: Remove redundant fallback operator.The expression
type || undefinedis redundant sincetypeis alreadyundefinedwhen falsy. Simplify to justtype.♻️ Simplification
- type: type || undefined, + type,.github/workflows/release-action.yml (2)
23-27: Considerfetch-depth: 0for tag operations/reproducibility.You’re creating/updating tags later; a full fetch reduces surprises with shallow history/tags.
Proposed tweak
- name: Checkout uses: actions/checkout@v4 with: ref: ${{ github.event.inputs.tag || (github.event.release.tag_name || github.ref) }} + fetch-depth: 0
24-24: Pin 3rd-party actions to commit SHAs for supply-chain hardening.The workflow currently uses version tags (
@v4,@v2) for actions/checkout, oven-sh/setup-bun, and softprops/action-gh-release. GitHub recommends pinning to full commit SHAs to prevent tag rewriting and ensure workflow integrity:
actions/checkout@v4→actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744oven-sh/setup-bun@v2→oven-sh/setup-bun@b7a1c7ccf290d58743029c4f6903da283811b979softprops/action-gh-release@v2→softprops/action-gh-release@a06a81a(v2.5.0)Also applies to: 29-29, 111-111
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/release-action.ymlpackages/action/action.ymlpackages/action/package.jsonpackages/action/src/benchmark.tspackages/action/src/inputs.tspackages/action/src/outputs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/action/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/outputs.tspackages/action/src/benchmark.tspackages/action/src/inputs.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/outputs.tspackages/action/src/benchmark.tspackages/action/src/inputs.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/src/outputs.tspackages/action/src/benchmark.tspackages/action/src/inputs.tspackages/action/action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/action/src/outputs.tspackages/action/src/benchmark.tspackages/action/src/inputs.tspackages/action/action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
packages/action/src/benchmark.tspackages/action/action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/action/action.yml
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
.github/workflows/release-action.yml
🧬 Code graph analysis (3)
packages/action/src/outputs.ts (1)
packages/action/src/types.ts (2)
MinifyResult(35-42)BenchmarkResult(54-61)
packages/action/src/benchmark.ts (3)
packages/action/src/types.ts (1)
ActionInputs(7-23)examples/server.js (1)
result(27-27)packages/benchmark/src/index.ts (1)
benchmark(16-20)
packages/action/src/inputs.ts (3)
packages/action/src/types.ts (1)
ActionInputs(7-23)packages/utils/src/index.ts (1)
isBuiltInCompressor(44-44)packages/utils/src/compressor-resolver.ts (1)
isBuiltInCompressor(266-268)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
🔇 Additional comments (6)
packages/action/action.yml (1)
1-91: LGTM! Well-structured GitHub Action manifest.The action manifest is comprehensive and aligns well with the implementation. The inputs and outputs are clearly defined with appropriate defaults, and the inclusion of oxc in the default benchmark compressors reflects the modernization efforts in this PR.
packages/action/src/outputs.ts (1)
1-49: LGTM! Clean and defensive output handling.The implementation correctly:
- Formats the reduction percentage to two decimal places
- Defensively aggregates gzip sizes only when present
- Conditionally publishes optional benchmark fields to avoid empty outputs
The code follows all TypeScript and formatting guidelines.
packages/action/src/inputs.ts (2)
76-86: Good input validation with clear error messages.The minReduction parsing correctly validates numeric input and provides a helpful error message specifying the expected range (0-100). The use of
Number.parseFloatwithNumber.isNaNcheck is appropriate.
102-114: Effective use of utilities per repository learnings.The function correctly uses
isBuiltInCompressorfrom@node-minify/utilsfor dynamic compressor resolution, as per the project's established patterns. The deprecation warnings provide clear migration guidance.Based on learnings, this follows the repository's pattern for compressor resolution.
.github/workflows/release-action.yml (2)
3-12: Trigger/input wiring looks right.
workflow_dispatchrequiringinputs.tagpairs well with publishing from a specific release tag.
45-73: Version extraction/validation is solid (and safely constrained).
Greptile SummaryThis PR successfully introduces a production-ready GitHub Action package that replaces the legacy composite action with a bundled Node.js 20 runtime action published to GitHub Marketplace. Key Implementation Highlights:
Architecture Quality:
Migration Strategy:
Documentation:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as GitHub Workflow
participant Action as node-minify Action
participant Core as @actions/core
participant Inputs as inputs.ts
participant Minify as minify.ts
participant NodeMinify as @node-minify/core
participant Benchmark as benchmark.ts
participant BenchPkg as @node-minify/benchmark
participant Reporters as reporters/*
participant Checks as checks.ts
User->>Action: Trigger action (uses: srod/node-minify@v1)
Action->>Core: index.ts::run()
Core->>Inputs: parseInputs()
Inputs->>Core: Return ActionInputs
Core->>Inputs: validateCompressor()
Inputs-->>Core: Emit warnings if deprecated
Core->>Minify: runMinification(inputs)
Minify->>NodeMinify: resolveCompressor(name)
NodeMinify-->>Minify: Return compressor function
Minify->>NodeMinify: minify({ compressor, input, output })
NodeMinify-->>Minify: Write minified file
Minify->>Minify: getFileSize() & getFilesizeGzippedRaw()
Minify-->>Core: Return MinifyResult
Core->>Core: setMinifyOutputs(result)
Core->>Reporters: generateSummary(result)
Reporters->>Core: Add to GITHUB_STEP_SUMMARY
alt PR Context & report-pr-comment
Core->>Reporters: postPRComment(result, token)
Reporters->>Core: Post comment via GitHub API
end
alt report-annotations
Core->>Reporters: addAnnotations(result)
Reporters->>Core: Add file annotations
end
alt benchmark enabled
Core->>Benchmark: runBenchmark(inputs)
Benchmark->>Benchmark: Filter type-required compressors
Benchmark->>BenchPkg: benchmark({ input, compressors })
loop For each compressor
BenchPkg->>NodeMinify: minify() with compressor
BenchPkg->>BenchPkg: Measure time & gzip
end
BenchPkg-->>Benchmark: Return BenchmarkResult
Benchmark-->>Core: Return mapped result
Core->>Core: setBenchmarkOutputs(result)
Core->>Reporters: generateBenchmarkSummary(result)
end
Core->>Checks: checkThresholds(reduction, inputs)
alt Threshold violation
Checks-->>Core: Return error message
Core->>User: setFailed(error)
else Success
Core->>User: Return success with outputs
end
|
| compressors: fileResult.results.map((r) => ({ | ||
| compressor: r.compressor, | ||
| success: r.success, | ||
| size: r.sizeBytes, | ||
| reduction: r.reductionPercent, | ||
| gzipSize: r.gzipBytes, | ||
| timeMs: r.timeMs, |
There was a problem hiding this comment.
Type mismatch: r.gzipBytes is being mapped to gzipSize, but the benchmark package returns gzipBytes as a number (see packages/benchmark/src/types.ts line 31), while the action's BenchmarkCompressorResult type expects gzipSize?: number.
However, looking at the benchmark CompressorMetrics type more carefully, it has both:
gzipBytes?: number(line 31)gzipSize?: string(line 32) - human-readable format
But the mapping on line 55 reads from r.gzipBytes when it should read from r.gzipBytes (which is correct), BUT the field is being assigned to action's gzipSize which expects a number.
The actual issue: Line 169 in packages/benchmark/src/runner.ts sets metrics.gzipSize = await getFilesizeGzippedInBytes(...) which returns a string, not gzipBytes. The gzipBytes field is never set in the benchmark package.
Solution: The benchmark package needs to set gzipBytes as well, OR the action should map from gzipSize and parse it, OR (best) add a getFilesizeGzippedRaw call to get the numeric value.
Actually, checking packages/benchmark/src/runner.ts again - there's no gzipBytes being set, only gzipSize (the string). So this line will always map undefined to the action's gzipSize field.
| compressors: fileResult.results.map((r) => ({ | |
| compressor: r.compressor, | |
| success: r.success, | |
| size: r.sizeBytes, | |
| reduction: r.reductionPercent, | |
| gzipSize: r.gzipBytes, | |
| timeMs: r.timeMs, | |
| compressors: fileResult.results.map((r) => ({ | |
| compressor: r.compressor, | |
| success: r.success, | |
| size: r.sizeBytes, | |
| reduction: r.reductionPercent, | |
| gzipSize: r.gzipBytes, // Note: benchmark package doesn't set gzipBytes, need to add getFilesizeGzippedRaw | |
| timeMs: r.timeMs, | |
| error: r.error, | |
| })), |
The benchmark package should be updated to call both getFilesizeGzippedInBytes (string) and getFilesizeGzippedRaw (number) to populate both fields.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/benchmark.ts
Line: 50:56
Comment:
Type mismatch: `r.gzipBytes` is being mapped to `gzipSize`, but the benchmark package returns `gzipBytes` as a number (see `packages/benchmark/src/types.ts` line 31), while the action's `BenchmarkCompressorResult` type expects `gzipSize?: number`.
However, looking at the benchmark `CompressorMetrics` type more carefully, it has both:
- `gzipBytes?: number` (line 31)
- `gzipSize?: string` (line 32) - human-readable format
But the mapping on line 55 reads from `r.gzipBytes` when it should read from `r.gzipBytes` (which is correct), BUT the field is being assigned to action's `gzipSize` which expects a number.
**The actual issue:** Line 169 in `packages/benchmark/src/runner.ts` sets `metrics.gzipSize = await getFilesizeGzippedInBytes(...)` which returns a *string*, not `gzipBytes`. The `gzipBytes` field is never set in the benchmark package.
**Solution:** The benchmark package needs to set `gzipBytes` as well, OR the action should map from `gzipSize` and parse it, OR (best) add a `getFilesizeGzippedRaw` call to get the numeric value.
Actually, checking `packages/benchmark/src/runner.ts` again - there's no `gzipBytes` being set, only `gzipSize` (the string). So this line will always map `undefined` to the action's `gzipSize` field.
```suggestion
compressors: fileResult.results.map((r) => ({
compressor: r.compressor,
success: r.success,
size: r.sizeBytes,
reduction: r.reductionPercent,
gzipSize: r.gzipBytes, // Note: benchmark package doesn't set gzipBytes, need to add getFilesizeGzippedRaw
timeMs: r.timeMs,
error: r.error,
})),
```
The benchmark package should be updated to call both `getFilesizeGzippedInBytes` (string) and `getFilesizeGzippedRaw` (number) to populate both fields.
How can I resolve this? If you propose a fix, please make it concise.| - name: Install compressor packages | ||
| run: | | ||
| mkdir -p /tmp/compressors && cd /tmp/compressors | ||
| npm init -y | ||
| npm install @node-minify/terser @node-minify/esbuild | ||
|
|
||
| # Create node_modules and copy packages | ||
| mkdir -p $GITHUB_WORKSPACE/node_modules | ||
| cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ |
There was a problem hiding this comment.
The test workflow installs compressor packages externally via npm, which means it's not actually testing whether the bundled action works with its bundled dependencies.
The whole point of using bun build --bundle (see packages/action/package.json line 43) is to bundle all dependencies into dist/index.js so the action doesn't need to install packages at runtime. But this test defeats that purpose by installing the packages anyway.
The problem:
- Users will use the action expecting bundled dependencies to "just work"
- This test doesn't verify that - it installs packages externally
- If bundling fails or doesn't include a compressor correctly, this test won't catch it
Solution: Remove the npm install step to test the bundled action as users will actually use it:
| - name: Install compressor packages | |
| run: | | |
| mkdir -p /tmp/compressors && cd /tmp/compressors | |
| npm init -y | |
| npm install @node-minify/terser @node-minify/esbuild | |
| # Create node_modules and copy packages | |
| mkdir -p $GITHUB_WORKSPACE/node_modules | |
| cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: "20" |
If the bundled dependencies don't work, the test will fail (which is what we want - it will catch bundling issues).
Note: Native dependencies (like @swc/core, lightningcss, etc.) may not bundle correctly with Bun's bundler. You may need to either:
- Use
@vercel/nccinstead (see the unusedbuild:nccscript in package.json) - Keep the npm install but document that bundling doesn't work yet
- Test with a subset of JS-only compressors that bundle correctly
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/test-action.yml
Line: 67:75
Comment:
The test workflow installs compressor packages externally via npm, which means it's not actually testing whether the bundled action works with its bundled dependencies.
The whole point of using `bun build --bundle` (see `packages/action/package.json` line 43) is to bundle all dependencies into `dist/index.js` so the action doesn't need to install packages at runtime. But this test defeats that purpose by installing the packages anyway.
**The problem:**
1. Users will use the action expecting bundled dependencies to "just work"
2. This test doesn't verify that - it installs packages externally
3. If bundling fails or doesn't include a compressor correctly, this test won't catch it
**Solution:** Remove the npm install step to test the bundled action as users will actually use it:
```suggestion
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: "20"
```
If the bundled dependencies don't work, the test will fail (which is what we want - it will catch bundling issues).
**Note:** Native dependencies (like `@swc/core`, `lightningcss`, etc.) may not bundle correctly with Bun's bundler. You may need to either:
1. Use `@vercel/ncc` instead (see the unused `build:ncc` script in package.json)
2. Keep the npm install but document that bundling doesn't work yet
3. Test with a subset of JS-only compressors that bundle correctly
How can I resolve this? If you propose a fix, please make it concise.
packages/action/src/inputs.ts
Outdated
| } | ||
|
|
||
| export const validateJavaCompressor = validateCompressor; | ||
|
|
There was a problem hiding this comment.
The function name validateJavaCompressor is misleading - it's just an alias for validateCompressor which doesn't actually validate Java-specific requirements.
The issue:
- The old composite action (
.github/actions/node-minify/action.ymllines 75-80) had asetup-javastep that automatically installed Java for gcc/yui compressors - The new action removed that setup and relies on Java being pre-installed (see comment in
action.ymlline 19: "pre-installed on GitHub runners") - But
validateJavaCompressordoesn't check if Java is actually available - If users run this in custom environments (self-hosted runners, containers), gcc/yui will fail with cryptic errors
Solution options:
- Recommended: Check if Java is available and provide helpful error message:
| export function validateJavaCompressor(compressor: string): void { | |
| validateCompressor(compressor); | |
| if (JAVA_COMPRESSORS.includes(compressor)) { | |
| // Check if Java is available | |
| try { | |
| const result = execSync('java -version 2>&1', { encoding: 'utf8' }); | |
| if (!result) { | |
| warning(`⚠️ Compressor '${compressor}' requires Java, but 'java' command not found. On GitHub-hosted runners, Java is pre-installed. On self-hosted runners, please install Java 11+ first.`); | |
| } | |
| } catch { | |
| warning(`⚠️ Compressor '${compressor}' requires Java, but 'java' command not found. On GitHub-hosted runners, Java is pre-installed. On self-hosted runners, please install Java 11+ first.`); | |
| } | |
| } | |
| } |
- Or rename to
validateCompressorsince it doesn't do Java-specific validation
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/inputs.ts
Line: 116:116
Comment:
The function name `validateJavaCompressor` is misleading - it's just an alias for `validateCompressor` which doesn't actually validate Java-specific requirements.
**The issue:**
1. The old composite action (`.github/actions/node-minify/action.yml` lines 75-80) had a `setup-java` step that automatically installed Java for gcc/yui compressors
2. The new action removed that setup and relies on Java being pre-installed (see comment in `action.yml` line 19: "pre-installed on GitHub runners")
3. But `validateJavaCompressor` doesn't check if Java is actually available
4. If users run this in custom environments (self-hosted runners, containers), gcc/yui will fail with cryptic errors
**Solution options:**
1. **Recommended:** Check if Java is available and provide helpful error message:
```suggestion
export function validateJavaCompressor(compressor: string): void {
validateCompressor(compressor);
if (JAVA_COMPRESSORS.includes(compressor)) {
// Check if Java is available
try {
const result = execSync('java -version 2>&1', { encoding: 'utf8' });
if (!result) {
warning(`⚠️ Compressor '${compressor}' requires Java, but 'java' command not found. On GitHub-hosted runners, Java is pre-installed. On self-hosted runners, please install Java 11+ first.`);
}
} catch {
warning(`⚠️ Compressor '${compressor}' requires Java, but 'java' command not found. On GitHub-hosted runners, Java is pre-installed. On self-hosted runners, please install Java 11+ first.`);
}
}
}
```
2. **Or rename** to `validateCompressor` since it doesn't do Java-specific validation
How can I resolve this? If you propose a fix, please make it concise.| > - uses: srod/node-minify@main | ||
| > ``` |
There was a problem hiding this comment.
The migration instruction says @main but according to the PR description and documentation (docs/src/content/docs/github-action.md line 22), users should use @v1 for the new action.
Using @main is risky because:
- It always points to the latest code (can break without warning)
- GitHub's best practice is to pin to major version tags (
@v1,@v2, etc.) - The PR is specifically setting up
v1tag management inrelease-action.yml
Solution:
| > - uses: srod/node-minify@main | |
| > ``` | |
| > **This action is deprecated.** Please use the new bundled action instead: | |
| > | |
| > ```yaml | |
| > - uses: srod/node-minify@v1 | |
| > ``` |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/actions/node-minify/README.md
Line: 6:7
Comment:
The migration instruction says `@main` but according to the PR description and documentation (`docs/src/content/docs/github-action.md` line 22), users should use `@v1` for the new action.
Using `@main` is risky because:
1. It always points to the latest code (can break without warning)
2. GitHub's best practice is to pin to major version tags (`@v1`, `@v2`, etc.)
3. The PR is specifically setting up `v1` tag management in `release-action.yml`
**Solution:**
```suggestion
> **This action is deprecated.** Please use the new bundled action instead:
>
> ```yaml
> - uses: srod/node-minify@v1
> ```
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| ```yaml | ||
| - uses: srod/node-minify@main |
There was a problem hiding this comment.
Same issue - should use @v1 instead of @main:
| ```yaml | |
| - uses: srod/node-minify@main | |
| ```yaml | |
| - uses: srod/node-minify@v1 | |
| with: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/actions/node-minify/README.md
Line: 30:32
Comment:
Same issue - should use `@v1` instead of `@main`:
```suggestion
```yaml
- uses: srod/node-minify@v1
with:
```
How can I resolve this? If you propose a fix, please make it concise.| const result = await benchmark({ | ||
| input: inputPath, | ||
| compressors: inputs.benchmarkCompressors, | ||
| includeGzip: inputs.includeGzip, | ||
| type: inputs.type, | ||
| iterations: 1, | ||
| }); |
There was a problem hiding this comment.
When inputs.type is undefined, it gets passed to the benchmark runner, which then passes it as type: options.type as "js" | "css" to the minify() function (see packages/benchmark/src/runner.ts lines 129 and 145).
The problem:
- The default benchmark compressors include
esbuild(line 62 ininputs.ts) - esbuild requires the
typeparameter (see validation ininputs.tslines 43-46) - If a user runs
benchmark: truewithout specifyingtype, the benchmark will fail when it tries to run esbuild - Same issue applies to
lightningcssandyuiif they're in the compressor list
Example failure scenario:
- uses: srod/node-minify@v1
with:
input: "app.js"
output: "app.min.js"
compressor: "terser"
benchmark: "true" # Uses default: "terser,esbuild,swc,oxc"
# type not specified!The benchmark will fail when it hits esbuild because type is undefined.
Solution: Either:
- Filter out compressors that require
typewhentypeis not provided:
| const result = await benchmark({ | |
| input: inputPath, | |
| compressors: inputs.benchmarkCompressors, | |
| includeGzip: inputs.includeGzip, | |
| type: inputs.type, | |
| iterations: 1, | |
| }); | |
| const result = await benchmark({ | |
| input: inputPath, | |
| compressors: inputs.type | |
| ? inputs.benchmarkCompressors | |
| : inputs.benchmarkCompressors.filter(c => !['esbuild', 'lightningcss', 'yui'].includes(c)), | |
| includeGzip: inputs.includeGzip, | |
| type: inputs.type, | |
| iterations: 1, | |
| }); |
- Or throw a clear error if benchmark compressors need type but it's not provided
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/benchmark.ts
Line: 25:31
Comment:
When `inputs.type` is `undefined`, it gets passed to the benchmark runner, which then passes it as `type: options.type as "js" | "css"` to the `minify()` function (see `packages/benchmark/src/runner.ts` lines 129 and 145).
**The problem:**
1. The default benchmark compressors include `esbuild` (line 62 in `inputs.ts`)
2. esbuild **requires** the `type` parameter (see validation in `inputs.ts` lines 43-46)
3. If a user runs `benchmark: true` without specifying `type`, the benchmark will fail when it tries to run esbuild
4. Same issue applies to `lightningcss` and `yui` if they're in the compressor list
**Example failure scenario:**
```yaml
- uses: srod/node-minify@v1
with:
input: "app.js"
output: "app.min.js"
compressor: "terser"
benchmark: "true" # Uses default: "terser,esbuild,swc,oxc"
# type not specified!
```
The benchmark will fail when it hits esbuild because `type` is undefined.
**Solution:** Either:
1. Filter out compressors that require `type` when `type` is not provided:
```suggestion
const result = await benchmark({
input: inputPath,
compressors: inputs.type
? inputs.benchmarkCompressors
: inputs.benchmarkCompressors.filter(c => !['esbuild', 'lightningcss', 'yui'].includes(c)),
includeGzip: inputs.includeGzip,
type: inputs.type,
iterations: 1,
});
```
2. Or throw a clear error if benchmark compressors need type but it's not provided
How can I resolve this? If you propose a fix, please make it concise.| /*! node-minify action tests - MIT Licensed */ | ||
|
|
||
| import { describe, expect, test } from "vitest"; | ||
| import { checkThresholds } from "../src/checks.ts"; | ||
| import type { ActionInputs, FileResult, MinifyResult } from "../src/types.ts"; | ||
|
|
||
| describe("Action Types", () => { | ||
| test("ActionInputs has required fields", () => { | ||
| const inputs: ActionInputs = { | ||
| input: "src/app.js", | ||
| output: "dist/app.min.js", | ||
| compressor: "terser", | ||
| options: {}, | ||
| reportSummary: true, | ||
| reportPRComment: false, | ||
| reportAnnotations: false, | ||
| benchmark: false, | ||
| benchmarkCompressors: ["terser", "esbuild"], | ||
| failOnIncrease: false, | ||
| minReduction: 0, | ||
| includeGzip: true, | ||
| workingDirectory: ".", | ||
| }; | ||
|
|
||
| expect(inputs.input).toBe("src/app.js"); | ||
| expect(inputs.compressor).toBe("terser"); | ||
| }); | ||
|
|
||
| test("FileResult has all metrics", () => { | ||
| const result: FileResult = { | ||
| file: "app.js", | ||
| originalSize: 10000, | ||
| minifiedSize: 3000, | ||
| reduction: 70, | ||
| gzipSize: 1000, | ||
| timeMs: 50, | ||
| }; | ||
|
|
||
| expect(result.reduction).toBe(70); | ||
| expect(result.gzipSize).toBe(1000); | ||
| }); | ||
|
|
||
| test("MinifyResult aggregates file results", () => { | ||
| const result: MinifyResult = { | ||
| files: [ | ||
| { | ||
| file: "a.js", | ||
| originalSize: 5000, | ||
| minifiedSize: 1500, | ||
| reduction: 70, | ||
| timeMs: 25, | ||
| }, | ||
| { | ||
| file: "b.js", | ||
| originalSize: 5000, | ||
| minifiedSize: 1500, | ||
| reduction: 70, | ||
| timeMs: 25, | ||
| }, | ||
| ], | ||
| compressor: "terser", | ||
| totalOriginalSize: 10000, | ||
| totalMinifiedSize: 3000, | ||
| totalReduction: 70, | ||
| totalTimeMs: 50, | ||
| }; | ||
|
|
||
| expect(result.files).toHaveLength(2); | ||
| expect(result.totalReduction).toBe(70); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Threshold Logic", () => { | ||
| test("should detect size increase", () => { | ||
| const reduction = -5; | ||
| const inputs = { | ||
| failOnIncrease: true, | ||
| minReduction: 0, | ||
| }; | ||
|
|
||
| const error = checkThresholds(reduction, inputs); | ||
| expect(error).toContain("larger than original"); | ||
| expect(error).toContain("5.0% increase"); | ||
| }); | ||
|
|
||
| test("should detect insufficient reduction", () => { | ||
| const reduction = 30; | ||
| const inputs = { | ||
| failOnIncrease: false, | ||
| minReduction: 50, | ||
| }; | ||
|
|
||
| const error = checkThresholds(reduction, inputs); | ||
| expect(error).toContain("below minimum threshold"); | ||
| }); | ||
|
|
||
| test("should pass when reduction meets threshold", () => { | ||
| const reduction = 60; | ||
| const inputs = { | ||
| failOnIncrease: false, | ||
| minReduction: 50, | ||
| }; | ||
|
|
||
| const error = checkThresholds(reduction, inputs); | ||
| expect(error).toBeNull(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test suite only covers type definitions and the checkThresholds function, but doesn't test the main action workflow, reporters, benchmark integration, or input parsing. This leaves significant functionality untested.
Missing test coverage:
parseInputs()- especially JSON parsing, validation, defaultsrunMinification()- file operations, compressor resolution, error handlingrunBenchmark()- benchmark integration, result mapping- Reporter functions - summary generation, PR comments, annotations
- Output setters - verifying outputs are set correctly
- Error scenarios - missing files, invalid compressor, failed minification
Recommendation: Add integration tests that:
- Mock
@actions/corefunctions (getInput, setOutput, etc.) - Test the full workflow with mock compressors
- Verify error handling and edge cases
- Test reporter functions with sample data
This would catch issues like the type mismatch in benchmark.ts or the missing outputs in action.yml before they reach production.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/__tests__/action.test.ts
Line: 1:107
Comment:
The test suite only covers type definitions and the `checkThresholds` function, but doesn't test the main action workflow, reporters, benchmark integration, or input parsing. This leaves significant functionality untested.
**Missing test coverage:**
- `parseInputs()` - especially JSON parsing, validation, defaults
- `runMinification()` - file operations, compressor resolution, error handling
- `runBenchmark()` - benchmark integration, result mapping
- Reporter functions - summary generation, PR comments, annotations
- Output setters - verifying outputs are set correctly
- Error scenarios - missing files, invalid compressor, failed minification
**Recommendation:** Add integration tests that:
1. Mock `@actions/core` functions (getInput, setOutput, etc.)
2. Test the full workflow with mock compressors
3. Verify error handling and edge cases
4. Test reporter functions with sample data
This would catch issues like the type mismatch in benchmark.ts or the missing outputs in action.yml before they reach production.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
The issue: Solution: Only pass the Apply the same fix to line 141-147 (the actual benchmark iteration). Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/benchmark/src/runner.ts
Line: 125:131
Comment:
When `options.type` is `undefined`, it's cast to `"js" | "css"` and passed to `minify()`. For compressors that require `type` (esbuild, lightningcss, yui), this will cause the minify call to fail because those compressors explicitly check for the type field.
**The issue:**
`undefined as "js" | "css"` is a TypeScript cast that doesn't change the runtime value - it's still `undefined` at runtime. When the minify function receives `type: undefined`, compressors like esbuild will throw an error.
**Solution:** Only pass the `type` field if it's defined:
```suggestion
await minify({
compressor,
input: file,
output: warmupFile,
...(options.type && { type: options.type as "js" | "css" }),
options: options.compressorOptions,
});
```
Apply the same fix to line 141-147 (the actual benchmark iteration).
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/release-action.yml:
- Around line 74-92: The workflow currently force-updates the specific release
tag using the git tag -fa and git push origin "$VERSION" --force commands in the
"Commit and tag built action dist" step, which breaks tag immutability; fix it
by removing the force-rewrite of the specific release tag: ensure dist is
committed before creating the tag (commit built files, create the tag normally
without -f, and git push origin "$VERSION"), or instead create the tag from a
release branch that already contains the dist, or push only a new tag and avoid
--force; keep force-push behavior only for the major tag (v1) if desired.
🧹 Nitpick comments (1)
.github/workflows/release-action.yml (1)
104-117: Consider adding dist verification before artifact creation.The artifact creation and upload logic is correct, but consider adding a verification step before packaging to ensure:
- The dist bundle is valid JavaScript
- Required files exist in dist/
- The action.yml syntax is valid
This would catch build failures before publishing.
🔍 Suggested verification step
Add before line 104:
- name: Verify built action run: | # Check that dist exists and contains expected files if [ ! -f packages/action/dist/index.js ]; then echo "Error: dist/index.js not found" exit 1 fi # Verify action.yml syntax (requires yq or similar) if command -v yq &> /dev/null; then yq eval action.yml > /dev/null fi # Basic JS syntax check node --check packages/action/dist/index.js
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-action.ymlaction.ymlpackages/action/src/inputs.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/action/src/inputs.ts
- action.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
.github/workflows/release-action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 22.x)
🔇 Additional comments (5)
.github/workflows/release-action.yml (5)
3-13: LGTM! Proper trigger and concurrency configuration.The workflow correctly triggers on release publications and manual dispatch, and the concurrency key properly prevents duplicate releases for the same tag.
33-43: LGTM! Proper build sequence.The build steps correctly use
--frozen-lockfilefor reproducible builds and follow the proper dependency build order for the monorepo structure.
94-102: LGTM! Standard major version tag pattern.Force-updating the major version tag (e.g., v1) is the correct pattern for GitHub Actions, allowing users to reference
@v1and automatically get the latest v1.x.x release. Error handling is also properly implemented.
45-72: The version format validation is correct and intentional.The flexible regex
^v[0-9]+(\.[0-9]+)*$is designed to handle both semantic version releases (v1.2.3) and major version tracking tags (v1, v2). The CHANGELOG confirms all releases follow strict semver (10.3.0, 10.2.0, etc.), while the workflow's major version tag update step (line 94+) intentionally maintains major-only tags. This is standard practice for GitHub Actions and the regex correctly supports both use cases.
28-31: No action needed. Bun version 1.3.5 is the latest stable release as of January 2026.
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.inputs.tag || (github.event.release.tag_name || github.ref) }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/release-action.ymlRepository: srod/node-minify
Length of output: 4692
🏁 Script executed:
# Verify if there are any other workflows or documentation about expected usage
rg -l "workflow_dispatch|release-action" --type=yaml -g '.github/workflows/*.yml' | head -20Repository: srod/node-minify
Length of output: 208
🏁 Script executed:
# Check if there's a README or documentation explaining the workflow usage
fd -e md -e txt | xargs rg -l "release-action|workflow_dispatch" 2>/dev/null | head -10Repository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Let's check if there's any GitHub documentation or issues in the repo about this
fd -type f -name "*.md" | xargs cat | head -100Repository: srod/node-minify
Length of output: 230
🏁 Script executed:
# Check git history or any documentation about the release process
git log --oneline --all -20 2>/dev/null | head -20Repository: srod/node-minify
Length of output: 1386
🏁 Script executed:
# Look for any issue templates or contributing guides
fd -type f \( -name "CONTRIBUTING*" -o -name "RELEASE*" -o -name ".github/ISSUE*" \)Repository: srod/node-minify
Length of output: 230
🏁 Script executed:
# Fix the fd command - looking for markdown files properly
fd -name "*.md" | xargs cat 2>/dev/null | grep -A 10 -B 10 "release\|workflow" | head -50Repository: srod/node-minify
Length of output: 290
🏁 Script executed:
# Check the recent commit that mentions "release workflow tag handling"
git show cd975e5 --stat 2>/dev/null | head -50Repository: srod/node-minify
Length of output: 3019
🏁 Script executed:
# Check the actual changes made to release-action.yml in that commit
git show cd975e5:.github/workflows/release-action.yml 2>/dev/null | head -100Repository: srod/node-minify
Length of output: 3238
🏁 Script executed:
# Compare with current version - get the diff
git diff cd975e5^..cd975e5 -- .github/workflows/release-action.yml 2>/dev/null | head -80Repository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Check if there's a pre-release or tag validation step before this workflow is called
rg "tag|release" .github/workflows/ --type yaml -B 2 -A 2 | grep -E "if:|needs:|on:|name:" | head -30Repository: srod/node-minify
Length of output: 1033
🏁 Script executed:
# Look for documentation or comments in the workflow about expected usage
cat .github/workflows/release-action.yml | grep -E "^#|description"Repository: srod/node-minify
Length of output: 112
🏁 Script executed:
# Check if there's any issue discussion or PR that addresses this
git log --all --grep="tag\|checkout" --oneline 2>/dev/null | head -20Repository: srod/node-minify
Length of output: 1373
🏁 Script executed:
# Check the publish.yml workflow to see if tags are created there first
cat .github/workflows/publish.yml | head -80Repository: srod/node-minify
Length of output: 2266
🏁 Script executed:
# Check if there's any validation or documentation about when this workflow should be run
rg "workflow_dispatch\|release-action" --type md -A 5 -B 5 2>/dev/null | head -40Repository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Look at actual usage - check if there's a standard practice documented
fd "\.md" | xargs grep -l "release\|workflow" 2>/dev/null | head -5Repository: srod/node-minify
Length of output: 185
🏁 Script executed:
# Check the GitHub action documentation
cat docs/src/content/docs/github-action.md | head -200Repository: srod/node-minify
Length of output: 4640
Ensure the tag exists before checkout for workflow_dispatch events.
For workflow_dispatch, if the specified tag doesn't exist in the repository, the checkout step will fail. The tag creation logic (lines 74-92) runs after checkout, so it cannot help. Either ensure tags are created before manually triggering this workflow, or add validation/error handling for non-existent tags.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/action/__tests__/benchmark.test.ts:
- Around line 35-62: Replace the locally redefined TYPE_REQUIRED_COMPRESSORS in
benchmark.ts with the shared constant from inputs.ts: remove the local
TYPE_REQUIRED_COMPRESSORS declaration and add an import for
TYPE_REQUIRED_COMPRESSORS from "./inputs.ts" at the top of benchmark.ts, then
ensure any references in the file (e.g., the filtering logic used when building
the compressors list) use that imported symbol.
In @packages/action/src/benchmark.ts:
- Line 11: Remove the duplicate constant by importing TYPE_REQUIRED_COMPRESSORS
from inputs.ts instead of redefining it in benchmark.ts: ensure inputs.ts
exports TYPE_REQUIRED_COMPRESSORS (add an export if missing), then replace the
local declaration in benchmark.ts with an import statement that brings in
TYPE_REQUIRED_COMPRESSORS and update any references to use the imported symbol.
In @packages/action/src/inputs.ts:
- Line 11: The constant TYPE_REQUIRED_COMPRESSORS currently includes
"lightningcss" but that compressor does not require or use the type parameter;
remove "lightningcss" from the TYPE_REQUIRED_COMPRESSORS array so it only
contains compressors that need a type (e.g., "esbuild" and "yui"). Update the
declaration of TYPE_REQUIRED_COMPRESSORS to exclude "lightningcss" (leave
"esbuild" and "yui") so validation that enforces a type parameter won't
incorrectly reject runs using lightningcss.
🧹 Nitpick comments (6)
.github/workflows/test-action.yml (2)
67-78: Consider handling pre-existing node_modules more robustly.The
cp -rapproach works for a clean checkout, but could cause issues ifnode_modulesalready exists (e.g., from cached actions or future workflow changes). The current merge behavior may lead to version conflicts.🔧 Suggested improvement
# Create node_modules and copy packages mkdir -p $GITHUB_WORKSPACE/node_modules - cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ + # Use rsync to merge packages, preferring newly installed versions + rsync -a /tmp/compressors/node_modules/ $GITHUB_WORKSPACE/node_modules/Alternatively, install directly into the workspace if the bundling concern is addressed:
- name: Install compressor packages run: npm install @node-minify/terser @node-minify/esbuild --no-save
173-184: Duplicate compressor installation pattern.This follows the same pattern as the JS job. Consider extracting the compressor installation into a composite action or reusable workflow step if more test jobs are added, to reduce duplication and maintenance overhead.
packages/action/src/inputs.ts (1)
13-20: Consider usingwarnDeprecationutility for consistency.The manual
DEPRECATED_COMPRESSORSmapping works well, but consider leveraging thewarnDeprecationutility from@node-minify/utilsto maintain consistency across the codebase, as indicated in the coding guidelines.Based on learnings, deprecation warnings should use
warnDeprecationfromnode-minify/utils.packages/action/src/index.ts (1)
86-86: Add top-level error handling for the run invocation.The
run()call should either be awaited or include a.catch()handler to ensure any unhandled promise rejections are properly caught and don't cause unexpected process termination.Proposed fix
-run(); +run().catch((error) => { + console.error("Fatal error:", error); + process.exit(1); +});packages/action/__tests__/inputs.test.ts (1)
127-135: Consider making the deprecated compressor test more explicit.The test assumes
"babel-minify"exists inDEPRECATED_COMPRESSORS. While this is likely correct, the test could be more maintainable by either:
- Documenting that "babel-minify" is a known deprecated compressor, or
- Testing the deprecation logic more generically
💡 Alternative approach for better maintainability
Option 1: Add a comment documenting the assumption:
test("warns for deprecated compressor", () => { vi.mocked(isBuiltInCompressor).mockReturnValue(true); - + // "babel-minify" is a known deprecated compressor in DEPRECATED_COMPRESSORS validateCompressor("babel-minify");Option 2: Mock the DEPRECATED_COMPRESSORS lookup if it becomes a problem, though the current approach is acceptable for integration-style tests.
packages/action/src/benchmark.ts (1)
34-40: Consider conditionally including type property only when defined.While the current code filters out type-required compressors when
typeis undefined (lines 27-32), you could make the intent clearer by only including thetypeproperty in the benchmark options when it's defined.♻️ Optional refactor for clarity
- const result = await benchmark({ - input: inputPath, - compressors, - includeGzip: inputs.includeGzip, - type: inputs.type, - iterations: 1, - }); + const benchmarkOptions: { + input: string; + compressors: string[]; + includeGzip: boolean; + type?: "js" | "css"; + iterations: number; + } = { + input: inputPath, + compressors, + includeGzip: inputs.includeGzip, + iterations: 1, + }; + + if (inputs.type) { + benchmarkOptions.type = inputs.type; + } + + const result = await benchmark(benchmarkOptions);This makes it explicit that
typeis only passed when defined, matching the pattern mentioned in the commit messages.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/actions/node-minify/README.md.github/workflows/test-action.ymlpackages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.tspackages/action/__tests__/outputs.test.tspackages/action/src/benchmark.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/benchmark/src/runner.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/node-minify/README.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/__tests__/outputs.test.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.tspackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/__tests__/outputs.test.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.tspackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/action/__tests__/outputs.test.tspackages/action/__tests__/inputs.test.tspackages/action/__tests__/benchmark.test.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/action/src/index.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/action/__tests__/outputs.test.tspackages/action/__tests__/inputs.test.ts.github/workflows/test-action.ymlpackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
packages/action/__tests__/outputs.test.tspackages/action/__tests__/inputs.test.tspackages/action/__tests__/benchmark.test.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/__tests__/outputs.test.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.ts.github/workflows/test-action.ymlpackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/action/src/index.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.ts.github/workflows/test-action.ymlpackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
packages/action/src/index.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
packages/action/src/index.tspackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.tspackages/action/src/benchmark.ts
📚 Learning: 2026-01-09T07:41:29.707Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.707Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/action/src/index.tspackages/action/__tests__/inputs.test.ts.github/workflows/test-action.ymlpackages/benchmark/src/runner.tspackages/action/__tests__/benchmark.test.ts
🧬 Code graph analysis (5)
packages/action/__tests__/outputs.test.ts (2)
packages/action/src/types.ts (1)
MinifyResult(35-42)packages/action/src/outputs.ts (2)
setMinifyOutputs(17-31)setBenchmarkOutputs(38-49)
packages/action/src/inputs.ts (2)
packages/action/src/types.ts (1)
ActionInputs(7-23)packages/utils/src/compressor-resolver.ts (1)
isBuiltInCompressor(266-268)
packages/action/__tests__/inputs.test.ts (1)
packages/action/src/inputs.ts (2)
parseInputs(38-90)validateCompressor(101-113)
packages/benchmark/src/runner.ts (2)
packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(75-85)packages/utils/src/index.ts (1)
getFilesizeGzippedRaw(41-41)
packages/action/__tests__/benchmark.test.ts (2)
packages/action/src/types.ts (1)
ActionInputs(7-23)packages/benchmark/src/index.ts (1)
benchmark(16-20)
🪛 actionlint (1.7.10)
.github/workflows/test-action.yml
99-99: file "packages/action/dist/index.js" does not exist in "/home/jailuser/git". it is specified at "main" key in "runs" section in "node-minify" action
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 22.x)
🔇 Additional comments (34)
.github/workflows/test-action.yml (5)
5-14: LGTM!The trigger configuration appropriately includes the feature branch and relevant paths for the action package. Consider removing the
feature/github-action-packagebranch from push triggers after merging to develop.
18-47: Well-structured artifact-based build job.The build job correctly creates the action distribution before test jobs run. The static analysis warning about
packages/action/dist/index.jsnot existing is a false positive—the file is created during this build step and made available to downstream jobs via the artifact.
105-131: LGTM!The verification logic correctly uses raw size outputs and properly validates both file creation and reduction percentage using
bcfor floating-point comparison.
141-153: LGTM!The comparison table correctly uses raw byte values and formats them consistently with the "bytes" suffix.
235-241: LGTM!The summary job correctly gates on both test jobs completing successfully.
packages/benchmark/src/runner.ts (5)
12-12: LGTM: New utility import added correctly.The import of
getFilesizeGzippedRawis appropriate for populating thegzipBytesfield and aligns with the utility's public API exposure.
57-63: LGTM: Good documentation added.The JSDoc clearly describes the function's purpose, parameters, and return value.
130-130: LGTM: Conditional type parameter handling prevents undefined propagation.The conditional spread
...(options.type && { type: options.type as "js" | "css" })ensures that thetypeparameter is only passed tominifywhen it has a defined value, preventing issues with compressors that don't handle undefined type values.Also applies to: 146-146
169-172: LGTM: Dual gzip metrics provide both human-readable and numeric values.Adding
gzipBytesalongsidegzipSizeprovides both a human-readable string and a numeric value, which is useful for programmatic comparisons and calculations downstream (e.g., in action outputs).
73-78: oxc compressor is properly integrated and available.The
@node-minify/oxcpackage is correctly registered in the monorepo, implements theCompressortype, and is included in the compressor resolver. Adding it to the default benchmark compressors is safe and supported.packages/action/src/inputs.ts (7)
1-5: LGTM: Copyright header included.Including the copyright header is good practice, even though the coding guidelines specifically require it only for
index.tsfiles.
42-46: LGTM: Clear error for missing required type.The validation correctly enforces that compressors requiring a
typeparameter fail fast with a descriptive error message.
48-56: LGTM: Robust JSON parsing with error handling.The try-catch block properly handles invalid JSON in the
optionsinput and provides a clear error message including the invalid value.
58-61: LGTM: Consistent default compressors.The default
benchmarkCompressorslist["terser", "esbuild", "swc", "oxc"]matches the default inpackages/benchmark/src/runner.ts(lines 73-78), ensuring consistency.
75-85: LGTM: Thorough validation for min-reduction input.The IIFE validates that
min-reductionis a valid number in the range 0-100 and provides a clear error message for invalid values.
88-88: LGTM: Standard GitHub token fallback.Using
process.env.GITHUB_TOKENas a fallback is standard practice for GitHub Actions.
101-113: LGTM: Appropriate validation with informative warnings.The function correctly uses
isBuiltInCompressorfrom@node-minify/utils(per coding guidelines) and provides helpful warnings for deprecated or custom compressors without blocking execution.packages/action/src/index.ts (4)
29-50: LGTM: Well-structured minification flow.The sequential flow of parsing inputs, validating, minifying, setting outputs, and conditionally generating reports is clear and appropriate. Each step delegates to focused helper functions.
52-66: LGTM: Conditional benchmark execution.The benchmark flow correctly uses the parsed
benchmarkCompressorsfrom inputs, sets outputs, conditionally generates summaries, and logs the recommended compressor when available.
68-72: LGTM: Threshold checking after all processing.Checking thresholds after all processing is complete and using
setFailedwith early return ensures the action fails appropriately when configured thresholds are violated.
77-83: LGTM: Proper error handling pattern.The error handling correctly uses
if (error instanceof Error)checks as specified in the coding guidelines and properly callssetFailedto signal action failure.packages/action/__tests__/outputs.test.ts (3)
14-105: LGTM: Comprehensive test coverage for setMinifyOutputs.The tests cover basic outputs, conditional gzip output when present, and verify that gzip-size is not set when no file has gzipSize. The format verification (e.g., "70.00" for reduction-percent) ensures consistency.
107-150: LGTM: Complete test coverage for setBenchmarkOutputs.The tests verify conditional output behavior (outputs only set when values are present) and ensure
benchmark-jsonis always emitted regardless of whether recommendation values are available.
1-17: LGTM: Proper test setup and mocking.The mock setup correctly uses
vi.mockbefore importing the modules, andbeforeEachproperly resets mocks between tests, following Vitest best practices.packages/action/__tests__/benchmark.test.ts (4)
64-90: LGTM: Comprehensive coverage of compressor inclusion logic.This test verifies the complementary behavior where all compressors are included when
typeis provided, ensuring complete test coverage of the filtering logic.
92-137: LGTM: Thorough verification of result mapping.The test comprehensively verifies that benchmark results are correctly mapped from the benchmark package's format to the action's
BenchmarkResultstructure, including field name transformations and summary values.
139-155: LGTM: Edge case coverage for empty results.The test ensures graceful handling when no benchmark files are returned, preventing potential undefined reference errors downstream.
1-33: LGTM: Well-structured test setup.The mock configuration and
baseInputsfixture provide a clean foundation for test cases, reducing duplication and improving maintainability.packages/action/__tests__/inputs.test.ts (3)
1-20: LGTM! Clean test setup with proper mocking.The mock setup follows Vitest best practices by mocking external dependencies before importing the module under test. The imports follow the project's coding guidelines correctly.
21-61: LGTM! Well-structured test suite with proper setup/teardown.The test setup correctly establishes default mock behavior and uses appropriate lifecycle hooks. The basic inputs test validates core parsing functionality.
62-119: LGTM! Comprehensive error handling and edge case coverage.These tests thoroughly validate input parsing error conditions including JSON validation, type requirements, numeric range checks, and CSV parsing. The test cases align with the implementation in
inputs.ts.packages/action/src/benchmark.ts (3)
13-32: LGTM! Well-documented function with correct compressor filtering.The JSDoc clearly explains the function's purpose, and the filtering logic correctly excludes compressors that require a
typeparameter when none is provided. This aligns with the PR objectives to prevent benchmark failures.
42-54: LGTM! Proper defensive handling for missing results.The code correctly handles the edge case where no file results are returned by providing safe default values. This prevents downstream errors and ensures the action can continue gracefully.
56-81: LGTM! Correct result mapping with proper null handling.The mapping logic correctly transforms the benchmark package's result format into the action's
BenchmarkResulttype. The conversion of "N/A" strings toundefinedis appropriate, and the mapping ofbestPerformancetobestSpeedmaintains semantic clarity for action consumers.
Following GitHub best practices to pin to major version tags.
… type not provided Prevents benchmark failures when users run benchmark: true without specifying type, since esbuild/lightningcss/yui require it.
- Add tests for parseInputs validation, defaults, and error handling - Add tests for validateCompressor warnings - Add tests for setMinifyOutputs and setBenchmarkOutputs - Add tests for runBenchmark filtering and result mapping Increases test coverage from 6 to 24 tests.
Prevents passing undefined as type which causes errors for compressors that require it (esbuild, lightningcss, yui).
- lightningcss is a pure CSS compressor that doesn't need the type param - Deduplicate TYPE_REQUIRED_COMPRESSORS by importing from inputs.ts - Update action.yml descriptions to reflect accurate requirements
- Validate type input explicitly instead of unsafe cast (js/css only) - Parse benchmark-compressors with deduplication and empty filtering - Don't leak raw JSON in options parse error messages - Add 7 new edge case tests (31 total)
lightningcss is a CSS-only compressor that doesn't need the type param. Updated docs, action.yml, CLI, and AGENTS.md to reflect this.
srod
force-pushed
the
feature/github-action-package
branch
from
5650f17 to
250f688
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/actions/node-minify/README.md:
- Around line 9-16: Update the action README to explicitly document the legacy
input named "java-version": state whether it is still supported in the migrated
action, or document its replacement name (if any) and the mapping behavior; add
the input to the inputs table/section with its accepted values, default value
(if applicable), an example usage snippet, and a deprecation note/instructions
for migration if it was removed or renamed; reference the unique input
identifier "java-version" and the README file sections that enumerate inputs
(e.g., the inputs table in packages/action/README.md) so readers can find the
change easily.
In @packages/action/package.json:
- Line 43: The build script in package.json currently silences failures when
copying src/index.d.ts (the "build" script uses "cp src/index.d.ts
dist/index.d.ts 2>/dev/null || true"), which can leave dist/index.d.ts missing
while the package "types" field still points to it; change the build step to
either generate TypeScript declarations (enable declaration emit during build)
or remove the redirection/|| true so the copy fails the build if src/index.d.ts
is absent, and ensure the "types" field matches the emitted declaration path
(src/index.d.ts / dist/index.d.ts or update it accordingly).
In @packages/action/src/types.ts:
- Around line 7-23: The ActionInputs.type union is too restrictive ("js" |
"css") while the action supports HTML, JSON and images; expand the type union on
ActionInputs to include "html", "json" and "image" (e.g., type?: "js" | "css" |
"html" | "json" | "image") and update any runtime validation that rejects
non-js/css values (including logic around esbuild/yui) to accept these values or
to only enforce the restriction when the chosen compressor truly requires it;
alternatively, if you prefer not to change the type, update README/docs and the
validation to explicitly state that type is only for esbuild/yui and that other
compressors infer type automatically.
In @packages/utils/src/getFilesizeGzippedInBytes.ts:
- Around line 38-45: The source read stream created in getFilesizeGzippedInBytes
may emit errors that don't propagate through pipe(), so attach an explicit error
handler to the source (e.g., source.on("error", reject)) and also ensure you
remove/destroy listeners/streams on resolve/reject to avoid leaks; keep the
existing gzip stream handlers (gzipSizeStream().on("gzip-size",
resolve).on("error", reject)) but add cleanup logic that removes event listeners
and destroys the source and gzip stream when the promise settles.
🧹 Nitpick comments (10)
scripts/check-published.ts (1)
72-93: Consider makingmainsynchronous since it doesn't useawait.The function is marked
asyncbut contains noawaitexpressions. Since all operations are synchronous, the function signature could be simplified.♻️ Suggested refactor
-async function main() { +function main() {And update the invocation at line 119:
-main().catch((err) => { +try { + main(); +} catch (err) {.github/workflows/test-action.yml (2)
71-78: Consider usingcp -rLto handle symlinks in node_modules.The
cp -rcommand may not correctly handle symlinks, which are common innode_modulesdue to npm's hoisting behavior. Usingcp -rL(dereference symlinks) would be more robust.✏️ Suggested fix
# Create node_modules and copy packages mkdir -p $GITHUB_WORKSPACE/node_modules - cp -r /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/ + cp -rL /tmp/compressors/node_modules/* $GITHUB_WORKSPACE/node_modules/
176-183: Same symlink handling improvement applies here.Consider using
cp -rLinstead ofcp -rfor consistency with the suggested fix in the JS minification job..github/actions/node-minify/README.md (1)
1-16: Add removal timeline and compatibility notes.The deprecation notice is clear, but consider adding:
- A timeline or target version for removal of the legacy action
- Whether there are breaking changes between the legacy and new action
- A note about version pinning (users on pinned commits won't see this deprecation warning)
📝 Suggested addition
After line 7, consider adding:
> ```yaml > - uses: srod/node-minify@v1 > ``` +> +> **Removal timeline:** This legacy action will be removed in v2.0 (planned Q2 2026). +> +> **Breaking changes:** The new action is fully compatible with existing workflows. All inputs and outputs remain the same.packages/utils/src/getFilesizeGzippedInBytes.ts (1)
54-65: Consider avoiding double-wrapping ofFileOperationError.Both public functions catch errors and wrap them in
FileOperationError, butgetGzipSizealready throwsFileOperationErrorfor validation failures. This results in nested error messages like "Failed to get gzipped size of file X: Failed to access file X: File does not exist".You could check if the caught error is already a
FileOperationErrorand re-throw it directly, or let validation errors propagate unwrapped and only wrap stream errors.♻️ Example fix for one function
export async function getFilesizeGzippedRaw(file: string): Promise<number> { try { return await getGzipSize(file); } catch (error) { + if (error instanceof FileOperationError) { + throw error; + } throw new FileOperationError( "get gzipped size of", file, error as Error ); } }Also applies to: 75-85
packages/action/src/minify.ts (1)
19-22: Consider usinggetFilesizeInBytesfrom@node-minify/utilsfor consistency.The coding guidelines specify using file operation utilities from
@node-minify/utils, includinggetFilesizeInBytes. Using the shared utility would maintain consistency across the codebase.♻️ Suggested refactor
-import { stat } from "node:fs/promises"; import { resolve } from "node:path"; import { minify } from "@node-minify/core"; -import { getFilesizeGzippedRaw, resolveCompressor } from "@node-minify/utils"; +import { + getFilesizeGzippedRaw, + getFilesizeInBytes, + resolveCompressor, +} from "@node-minify/utils"; import type { ActionInputs, FileResult, MinifyResult } from "./types.ts"; -/** - * Get the size of a file in bytes. - * - * @param filePath - Path to the file - * @returns The file size in bytes - */ -async function getFileSize(filePath: string): Promise<number> { - const stats = await stat(filePath); - return stats.size; -}Then replace
getFileSizecalls withgetFilesizeInBytes:- const originalSize = await getFileSize(inputPath); + const originalSize = await getFilesizeInBytes(inputPath);- const minifiedSize = await getFileSize(outputPath); + const minifiedSize = await getFilesizeInBytes(outputPath);Based on coding guidelines, use file operation utilities from
@node-minify/utils.packages/action/__tests__/outputs.test.ts (1)
49-78: Consider adding a test case for summing multiple gzip sizes.The current test verifies gzip-size is emitted when any file has
gzipSize, but the implementation inoutputs.ts(lines 23-27) sums all gzip sizes. Consider adding a test where multiple files havegzipSizeto verify the summing behavior:🧪 Additional test case
test("sums gzip-size across multiple files", () => { const result: MinifyResult = { files: [ { file: "a.js", originalSize: 5000, minifiedSize: 1500, reduction: 70, gzipSize: 600, timeMs: 25, }, { file: "b.js", originalSize: 5000, minifiedSize: 1500, reduction: 70, gzipSize: 800, timeMs: 25, }, ], compressor: "terser", totalOriginalSize: 10000, totalMinifiedSize: 3000, totalReduction: 70, totalTimeMs: 50, }; setMinifyOutputs(result); expect(setOutput).toHaveBeenCalledWith("gzip-size", 1400); });packages/action/__tests__/action.test.ts (2)
7-71: Type validation tests look good; consider expanding edge case coverage.The type tests verify the basic structure of
ActionInputs,FileResult, andMinifyResult. These serve as both compile-time type checking and runtime sanity checks.Consider adding a test case for the optional
brotliSizefield inFileResultto match the type definition.
73-107: Threshold logic tests cover key scenarios well.The tests validate size increase detection, insufficient reduction, and passing thresholds correctly. Consider adding boundary tests for:
reduction = 0withfailOnIncrease = true(boundary condition)reductionexactly equal tominReduction(should pass)packages/action/src/reporters/summary.ts (1)
19-49: Summary generation looks good;brotliSizeis omitted.The
FileResultinterface includes an optionalbrotliSizefield, but it's not displayed in the summary table. If brotli metrics are relevant, consider adding a column, or document that it's intentionally excluded for brevity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.changeset/fluffy-eagles-sing.mdis excluded by!.changeset/**bun.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.github/actions/node-minify/README.md.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts.github/workflows/release-action.yml.github/workflows/test-action.ymlAGENTS.mdaction.ymldocs/src/consts.tsdocs/src/content/docs/benchmark.mddocs/src/content/docs/cli.mddocs/src/content/docs/github-action.mdpackages/action/README.mdpackages/action/__tests__/action.test.tspackages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.tspackages/action/__tests__/outputs.test.tspackages/action/action.ymlpackages/action/package.jsonpackages/action/src/benchmark.tspackages/action/src/checks.tspackages/action/src/index.d.tspackages/action/src/index.tspackages/action/src/inputs.tspackages/action/src/minify.tspackages/action/src/outputs.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/comment.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/tsconfig.jsonpackages/action/vitest.config.tspackages/benchmark/README.mdpackages/benchmark/src/runner.tspackages/cli/__tests__/cli.test.tspackages/cli/src/bin/cli.tspackages/utils/__tests__/utils.test.tspackages/utils/src/compressor-resolver.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/utils/src/index.tsscripts/check-published.ts
✅ Files skipped from review due to trivial changes (1)
- packages/action/README.md
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/utils/src/compressor-resolver.ts
- AGENTS.md
- packages/action/src/index.d.ts
- packages/action/src/inputs.ts
- packages/action/src/reporters/comment.ts
- docs/src/content/docs/github-action.md
- packages/action/src/index.ts
- packages/utils/tests/utils.test.ts
- packages/action/tests/benchmark.test.ts
- packages/action/tsconfig.json
- packages/action/tests/inputs.test.ts
- packages/cli/tests/cli.test.ts
- packages/action/src/outputs.ts
- packages/cli/src/bin/cli.ts
- packages/benchmark/README.md
- packages/action/src/benchmark.ts
- .github/actions/node-minify/action.yml
- action.yml
- docs/src/consts.ts
- packages/benchmark/src/runner.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/src/checks.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/action/vitest.config.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/__tests__/outputs.test.tspackages/action/__tests__/action.test.tsscripts/check-published.tspackages/utils/src/index.tspackages/action/src/minify.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/src/checks.tspackages/utils/src/getFilesizeGzippedInBytes.tspackages/action/vitest.config.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/__tests__/outputs.test.tspackages/action/__tests__/action.test.tsscripts/check-published.tspackages/utils/src/index.tspackages/action/src/minify.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/action/__tests__/outputs.test.tspackages/action/__tests__/action.test.ts
packages/*/src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Include copyright header
/*! node-minify ... MIT Licensed */in source files
Files:
packages/utils/src/index.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Applied to files:
.github/actions/node-minify/README.mdpackages/action/src/reporters/annotations.tsdocs/src/content/docs/benchmark.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
.github/actions/node-minify/README.mdpackages/utils/src/getFilesizeGzippedInBytes.ts.github/workflows/test-action.yml.github/actions/node-minify/minify.tspackages/action/src/reporters/annotations.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/__tests__/outputs.test.tspackages/action/action.ymldocs/src/content/docs/benchmark.mdpackages/action/__tests__/action.test.tspackages/utils/src/index.tspackages/action/src/minify.tsdocs/src/content/docs/cli.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
.github/actions/node-minify/README.mdpackages/utils/src/getFilesizeGzippedInBytes.ts.github/workflows/test-action.yml.github/actions/node-minify/minify.tspackages/action/src/reporters/summary.tspackages/action/src/types.tspackages/action/__tests__/outputs.test.tspackages/action/action.ymldocs/src/content/docs/benchmark.mdpackages/utils/src/index.tspackages/action/src/minify.tsdocs/src/content/docs/cli.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
.github/actions/node-minify/README.md.github/workflows/test-action.yml.github/actions/node-minify/minify.tspackages/action/action.ymldocs/src/content/docs/benchmark.mddocs/src/content/docs/cli.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
.github/actions/node-minify/README.mdpackages/utils/src/getFilesizeGzippedInBytes.ts.github/workflows/test-action.yml.github/actions/node-minify/minify.tspackages/action/src/reporters/annotations.tspackages/action/src/types.tspackages/action/action.ymldocs/src/content/docs/benchmark.mdpackages/action/__tests__/action.test.tspackages/utils/src/index.tspackages/action/src/minify.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
.github/workflows/test-action.yml.github/actions/node-minify/minify.tspackages/action/__tests__/outputs.test.tsdocs/src/content/docs/benchmark.mdpackages/action/__tests__/action.test.tsscripts/check-published.tspackages/utils/src/index.tsdocs/src/content/docs/cli.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
packages/action/vitest.config.tspackages/action/__tests__/outputs.test.tspackages/action/__tests__/action.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/*/src/index.ts : Include copyright header `/*! node-minify ... MIT Licensed */` in source files
Applied to files:
.github/actions/node-minify/minify.tsdocs/src/content/docs/benchmark.mdpackages/action/__tests__/action.test.tspackages/action/src/minify.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx} : Use PascalCase for type and interface names
Applied to files:
packages/action/src/types.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `node:` prefix for Node.js built-in imports (e.g., `node:child_process`)
Applied to files:
docs/src/content/docs/benchmark.md
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: All packages follow standard structure: `src/index.ts`, `__tests__/`, `package.json`, `tsconfig.json`, built with `tsdown`
Applied to files:
packages/action/__tests__/action.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `try/catch` with `if (err instanceof Error)` checks for error handling
Applied to files:
scripts/check-published.tspackages/action/src/minify.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Promise.all` for parallel operations where all must succeed, or `Promise.allSettled` when you need results regardless of individual failures
Applied to files:
scripts/check-published.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Use Bun as the package manager and runtime for this repository (Node.js >=20.0.0, Bun 1.3.5+)
Applied to files:
scripts/check-published.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Applied to files:
packages/utils/src/index.ts.github/workflows/release-action.yml
🧬 Code graph analysis (6)
packages/action/src/checks.ts (1)
packages/action/src/types.ts (1)
ActionInputs(7-23)
packages/utils/src/getFilesizeGzippedInBytes.ts (3)
packages/utils/src/error.ts (1)
FileOperationError(11-25)packages/utils/src/index.ts (3)
isValidFile(46-46)getFilesizeGzippedInBytes(40-40)getFilesizeGzippedRaw(41-41)packages/utils/src/isValidFile.ts (1)
isValidFile(21-27)
packages/action/src/reporters/annotations.ts (1)
packages/action/src/types.ts (1)
MinifyResult(35-42)
packages/action/__tests__/outputs.test.ts (2)
packages/action/src/types.ts (1)
MinifyResult(35-42)packages/action/src/outputs.ts (2)
setMinifyOutputs(17-31)setBenchmarkOutputs(38-49)
packages/action/__tests__/action.test.ts (2)
packages/action/src/types.ts (3)
ActionInputs(7-23)FileResult(25-33)MinifyResult(35-42)packages/action/src/checks.ts (1)
checkThresholds(18-31)
packages/action/src/minify.ts (4)
packages/action/src/types.ts (3)
ActionInputs(7-23)MinifyResult(35-42)FileResult(25-33)packages/utils/src/index.ts (2)
resolveCompressor(52-52)getFilesizeGzippedRaw(41-41)packages/core/src/index.ts (1)
minify(21-27)packages/utils/src/getFilesizeGzippedInBytes.ts (1)
getFilesizeGzippedRaw(75-85)
🪛 actionlint (1.7.10)
.github/workflows/test-action.yml
99-99: file "packages/action/dist/index.js" does not exist in "/home/jailuser/git". it is specified at "main" key in "runs" section in "node-minify" action
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 20.x)
🔇 Additional comments (42)
scripts/check-published.ts (4)
1-3: LGTM!Imports correctly use the
node:prefix for Node.js built-in modules as per coding guidelines. UsingexecFileSyncinstead ofexecSyncis a good security practice as it avoids shell interpolation.
7-19: LGTM!The function is well-documented and correctly filters package directories that contain a
package.jsonfile. The use ofwithFileTypesfor efficient directory filtering is a good practice.
28-61: Good security practices with input validation andexecFileSync.The input validation regex and use of
execFileSyncwith argument arrays effectively prevent command injection. The function structure and return type are clear.Minor note: The catch block at line 58 doesn't follow the coding guideline pattern of
if (err instanceof Error)for error handling. However, since the function simply returns a fallback value on any error, this is acceptable for this use case.
119-127: LGTM!The error handling wrapper correctly follows the coding guideline pattern of
if (err instanceof Error)checks, provides appropriate error logging with stack traces, and exits with a non-zero code for CI/CD compatibility.packages/action/vitest.config.ts (2)
3-7: Add trailing comma per coding guidelines.The ES5 trailing comma convention requires a trailing comma after the last property in objects and arrays.
✏️ Suggested fix
export default defineProject({ test: { - testTimeout: 30000, + testTimeout: 30000, + }, - }, });Or more precisely:
test: { testTimeout: 30000, - }, + }, });Wait, let me correct that:
export default defineProject({ test: { testTimeout: 30000, - }, + }, });Actually, looking more carefully - the inner object has a trailing comma on line 5, but the outer
testproperty on line 6 is missing one:export default defineProject({ test: { testTimeout: 30000, - }, + }, });Hmm, line 6 does have a trailing comma. Let me re-check:
},- yes it does. The configuration looks correct.
1-7: LGTM!The Vitest configuration is well-structured with appropriate settings. The 30-second timeout is reasonable for action tests involving actual minification operations. Code follows the coding guidelines (4-space indentation, double quotes, semicolons, trailing commas).
.github/workflows/test-action.yml (3)
18-47: LGTM! Well-structured build job.The build-action job correctly sets up Bun, builds dependencies in the right order (deps → core → benchmark → action), and uploads the dist artifact. The 1-day retention is appropriate for ephemeral CI artifacts.
97-131: LGTM!The terser minification test is well-structured with proper output verification. The
uses: ./correctly references the root action.yml. The static analysis warning about missingpackages/action/dist/index.jsis a false positive—the file is built by thebuild-actionjob and downloaded via artifact before this step executes.
234-240: LGTM!The summary job correctly aggregates the test results through job dependencies. The simple success message is appropriate for this aggregation pattern.
.github/actions/node-minify/README.md (1)
41-86: Good practice preserving legacy documentation.Maintaining the complete legacy documentation during the deprecation period helps users who haven't migrated yet. This is the correct approach for a gradual deprecation.
packages/utils/src/getFilesizeGzippedInBytes.ts (1)
1-11: LGTM!Copyright header present, imports use
node:prefix and.tsextensions per coding guidelines.packages/utils/src/index.ts (1)
16-19: LGTM!The new
getFilesizeGzippedRawexport is correctly imported and re-exported, maintaining the existing alphabetical ordering and patterns. The public API surface expansion aligns with the PR objectives.Also applies to: 40-41
docs/src/content/docs/cli.md (2)
108-108: Default compressors table updated correctly.The options table accurately reflects the new default compressor list including oxc, maintaining consistency with the example above and the benchmark documentation.
81-81: Documentation update correctly reflects the implementation.The example command
node-minify benchmark src/app.js --compressors terser,esbuild,swc,oxcmatches the benchmark function's default compressors list, which is hardcoded as["terser", "esbuild", "swc", "oxc"]in the implementation. The oxc compressor is properly registered in the CLI configuration and included as a dependency, confirming the change aligns with the PR objectives.docs/src/content/docs/benchmark.md (4)
31-31: CLI example updated consistently.The benchmark CLI example correctly includes oxc in the compressor list, maintaining consistency with the CLI documentation at docs/src/content/docs/cli.md.
87-87: Programmatic API example updated correctly.The code example properly demonstrates using oxc in the compressors array, helping developers understand how to use the new default compressor programmatically.
127-127: Options reference table updated correctly.The default compressors value in the options reference accurately reflects the new defaults including oxc, maintaining consistency across all documentation examples and reference sections.
54-54: The documentation change is accurate. Verification confirms that esbuild requires explicit type specification (throws error ifsettings.typeis missing or invalid), while lightningcss uses a hardcoded filename ("input.css") and does not require type specification. The updated comment correctly reflects this distinction..github/actions/node-minify/minify.ts (8)
5-8: LGTM!Good use of the shared
resolveCompressorutility from@node-minify/utilsfor dynamic compressor resolution. Based on learnings, this aligns with the project's recommended approach.
10-19: LGTM!Clean interface definition with appropriate nullable types for optional gzip metrics.
27-36: LGTM!The byte formatting logic is correct with proper edge case handling and overflow protection.
45-51: LGTM!Standard GitHub Actions output pattern. The error throw is appropriate since this action is designed to run in the GitHub Actions environment.
58-61: LGTM!Simple wrapper with error propagation to the caller's try/catch block.
204-208: LGTM!The outer catch provides a safety net for any unhandled rejections that might escape the inner try/catch block. While slightly redundant given the comprehensive error handling in
run(), it's a reasonable defensive pattern for a CI action.
148-166: No action needed — the code correctly uses the API.The function
getFilesizeGzippedInBytesreturns a formatted string (e.g.,"44 B"), not a numeric byte count. The code in lines 148-166 properly parses this formatted string with regex to extract the value and unit, then converts it back to bytes. This is the intended behavior and matches the function's implementation inpackages/utils/src/getFilesizeGzippedInBytes.ts.
112-120: No changes required. The removal oflightningcssfrom therequiresTypecheck is correct.lightningcssis a CSS-only minifier that does not require or use a type parameter—it hardcodes the input filename as"input.css"and always treats input as CSS. Unlikeesbuildandyui, which support both JS and CSS and need the type parameter to determine which minification rules to apply,lightningcsshas no need for type disambiguation.packages/action/action.yml (1)
1-93: LGTM!The action manifest is well-structured with comprehensive input/output declarations, sensible defaults, and clear documentation. The branding and node20 runtime configuration are appropriate for a GitHub Marketplace action.
packages/action/src/minify.ts (1)
39-91: LGTM!The minification logic is well-implemented:
- Correctly resolves paths relative to working directory
- Uses
resolveCompressorutility as per guidelines- Handles conditional options spreading cleanly
- Properly calculates reduction percentage with zero-check
- Returns a complete
MinifyResultmatching the interface.github/workflows/release-action.yml (1)
1-44: LGTM on workflow setup and build steps.The workflow is well-structured with:
- Appropriate triggers (release + manual dispatch)
- Concurrency control to prevent parallel releases
- Correct permissions scope
- Proper build sequence (dependencies → core → action)
packages/action/src/checks.ts (1)
18-31: LGTM!The threshold checking logic is correct and well-documented:
- Properly handles size increase detection when
failOnIncreaseis enabled- Validates minimum reduction threshold only when configured (> 0)
- Uses
Pick<>for precise type narrowing- Clear, human-readable error messages with formatted percentages
packages/action/__tests__/outputs.test.ts (1)
1-150: LGTM on test structure and coverage.The tests follow Vitest conventions correctly with proper mock setup,
beforeEachresets, and clear test descriptions. Coverage includes both positive and negative cases for output emission.packages/action/__tests__/action.test.ts (1)
1-6: LGTM! Proper test setup with copyright header and imports.The test file follows the project conventions with proper copyright header and uses Vitest as required by coding guidelines.
packages/action/package.json (1)
52-66: Dependencies look well-structured for the action package.The workspace dependencies and GitHub Actions libraries are appropriately declared. The inclusion of multiple compressor packages supports the benchmark feature.
packages/action/src/reporters/summary.ts (2)
1-9: LGTM! Proper imports and copyright header.The file correctly imports the GitHub Actions summary API and utility functions with proper copyright header.
58-108: LGTM! Benchmark summary with proper null handling and badges.The function correctly handles failed compressors, applies badges for recommended/best-speed/best-compression, and uses appropriate null checks for optional fields.
packages/action/src/reporters/annotations.ts (3)
1-11: LGTM! Clean imports and meaningful threshold constants.The constants for
LOW_REDUCTION_THRESHOLDandVERY_LOW_REDUCTION_THRESHOLDmake the code self-documenting.
23-45: LGTM! Well-structured annotation logic with clear thresholds.The graduated severity levels (error → warning → notice) based on reduction percentages provide actionable feedback. The boundary at 0% correctly distinguishes size increase (error) from no reduction (warning).
53-55: LGTM! Simple and focused error annotation utility.The function provides a clean API for reporting minification failures with file context.
packages/action/src/types.ts (4)
1-6: LGTM! Proper copyright header.The file follows the project's standard copyright header format.
25-42: LGTM! Well-structured result interfaces.
FileResultandMinifyResultprovide comprehensive metrics for individual files and aggregated results. The optional compression fields (gzipSize,brotliSize) allow flexibility for different reporting needs.
44-61: LGTM! Benchmark types handle success and failure states well.The
successboolean with optional metrics anderrorfield cleanly models both successful and failed compressor runs.
63-69: LGTM! Comparison result type supports new file detection.The nullable
baseSizeandchangefields with theisNewflag cleanly handle both existing and newly-added files in size comparisons.
| export interface ActionInputs { | ||
| input: string; | ||
| output: string; | ||
| compressor: string; | ||
| type?: "js" | "css"; | ||
| options: Record<string, unknown>; | ||
| reportSummary: boolean; | ||
| reportPRComment: boolean; | ||
| reportAnnotations: boolean; | ||
| benchmark: boolean; | ||
| benchmarkCompressors: string[]; | ||
| failOnIncrease: boolean; | ||
| minReduction: number; | ||
| includeGzip: boolean; | ||
| workingDirectory: string; | ||
| githubToken?: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for type field usage in the action directory
rg -n "type" packages/action/src/ --type ts --type tsx -A2 -B2 | head -100Repository: srod/node-minify
Length of output: 87
🏁 Script executed:
# Look for type constants or enums that might define supported types
rg -n "js|css|html|json|image" packages/action/src/ --type ts --type tsx | grep -i typeRepository: srod/node-minify
Length of output: 87
🏁 Script executed:
# Check the compressor definitions to understand type relationships
fd . packages/ -name "*.ts" -o -name "*.tsx" | head -20Repository: srod/node-minify
Length of output: 290
🏁 Script executed:
# Look at resolveCompressor utility mentioned in learnings
rg -n "resolveCompressor\|isBuiltInCompressor" packages/ -A5 -B2Repository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Check if there are type or interface definitions that define supported types
rg -n "export.*type.*=|enum.*type" packages/action/src/ -iRepository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Search for type field usage and compressor handling
rg "type.*=" packages/action/src/ -A2 -B2Repository: srod/node-minify
Length of output: 801
🏁 Script executed:
# Look for supported types or compressor definitions
rg "js|css|html|json|image" packages/ -i | grep -i "type\|compressor" | head -30Repository: srod/node-minify
Length of output: 2841
🏁 Script executed:
# Check ActionInputs type interface definition context
cat packages/action/src/types.tsRepository: srod/node-minify
Length of output: 1550
🏁 Script executed:
# Look for any enum or constant definitions related to types
rg "export.*\(type\|enum\|const\)" packages/action/src/ -A3Repository: srod/node-minify
Length of output: 42
🏁 Script executed:
# Search for actual usage of the type field in action logic
rg "\btype\b" packages/action/src/ --context 3Repository: srod/node-minify
Length of output: 10197
🏁 Script executed:
# Check if there's any mention of HTML, JSON, or image support in the action
rg "html|json|image" packages/action/ -iRepository: srod/node-minify
Length of output: 3707
🏁 Script executed:
# Check the core minify package for supported types
cat packages/core/src/index.ts | head -50Repository: srod/node-minify
Length of output: 897
🏁 Script executed:
# Look for type definitions in core or utilities
rg "type.*html|type.*json|type.*image" packages/ -iRepository: srod/node-minify
Length of output: 805
🏁 Script executed:
# Check if there are comments about supported file types
rg "supported.*type|type.*support" packages/ -i -A2 -B2 | head -50Repository: srod/node-minify
Length of output: 593
The type field should be expanded or documentation clarified regarding supported file types.
The action's description and README advertise support for minifying HTML, JSON, and images. However, the type field is restricted to "js" | "css" with explicit validation that rejects other values. While the optional type parameter is only required for esbuild and yui compressors (which support only JS and CSS), the broader compressor ecosystem in the action supports HTML (html-minifier, minify-html), JSON (jsonminify), and images (imagemin), yet these type values are not accepted. Either extend the type field to include "html" and "json" to match advertised capabilities, or clarify the documentation that type inference is automatic for non-esbuild/yui compressors and the type field is only for explicit type specification when needed.
🤖 Prompt for AI Agents
In @packages/action/src/types.ts around lines 7 - 23, The ActionInputs.type
union is too restrictive ("js" | "css") while the action supports HTML, JSON and
images; expand the type union on ActionInputs to include "html", "json" and
"image" (e.g., type?: "js" | "css" | "html" | "json" | "image") and update any
runtime validation that rejects non-js/css values (including logic around
esbuild/yui) to accept these values or to only enforce the restriction when the
chosen compressor truly requires it; alternatively, if you prefer not to change
the type, update README/docs and the validation to explicitly state that type is
only for esbuild/yui and that other compressors infer type automatically.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/node-minify/minify.ts (1)
149-164: UsegetFilesizeGzippedRawinstead of parsing the formatted string.
getFilesizeGzippedInBytesreturns a formatted string (e.g.,"1.5 kB"), not raw bytes. The current code parses this string via regex to convert it back to bytes, which is fragile and loses precision. The utility providesgetFilesizeGzippedRawthat returns the byte count directly—use it instead to avoid unnecessary string parsing.Example fix
- import { getFilesizeGzippedInBytes, resolveCompressor } from "@node-minify/utils"; + import { getFilesizeGzippedRaw, resolveCompressor } from "@node-minify/utils"; - if (includeGzip) { - const gzipResult = await getFilesizeGzippedInBytes(outputPath); - const match = gzipResult.match(/([\d.]+)\s*(\w+)/); - if (match) { - const value = Number.parseFloat(match[1]); - const unit = match[2]; - const multipliers: Record<string, number> = { - B: 1, - kB: 1024, - MB: 1024 * 1024, - GB: 1024 * 1024 * 1024, - }; - gzipSize = Math.round(value * (multipliers[unit] || 1)); - gzipSizeFormatted = gzipResult; - } - } + if (includeGzip) { + gzipSize = await getFilesizeGzippedRaw(outputPath); + gzipSizeFormatted = formatBytes(gzipSize); + }
🧹 Nitpick comments (2)
packages/action/__tests__/inputs.test.ts (1)
21-51: TheafterEachblock is redundant.Since
beforeEachalready callsvi.resetAllMocks()at line 23, theafterEachblock at lines 49-51 is unnecessary. Each test starts with a fresh mock state regardless.♻️ Proposed fix
}); - afterEach(() => { - vi.resetAllMocks(); - }); - test("parses basic inputs correctly", () => {packages/action/src/inputs.ts (1)
126-138: Consider usingwarnDeprecationfrom@node-minify/utilsfor deprecation warnings to maintain consistency with the core library's deprecation pattern.The
warnDeprecationutility is available in@node-minify/utilsand is already used elsewhere in the codebase for handling deprecations. It provides deduplication (warns once per package) and follows the established pattern. While@actions/core.warning()is valid for surfacing warnings in the GitHub Action UI, following the core library's deprecation approach would align with the coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/node-minify/minify.tspackages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.tspackages/action/src/inputs.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use 4 spaces for indentation
Use double quotes for strings
Always include semicolons at end of statements
Use ES5 trailing commas in objects and arrays
Usenode:prefix for Node.js built-in imports (e.g.,node:child_process)
Use camelCase for function and variable names
Usetry/catchwithif (err instanceof Error)checks for error handling
UsePromise.allfor parallel operations where all must succeed, orPromise.allSettledwhen you need results regardless of individual failures
Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion
Use file operation utilities from@node-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync
Use deprecation warnings viawarnDeprecationfrom@node-minify/utilsfor deprecated packages
UseresolveCompressorandisBuiltInCompressorutilities from@node-minify/utilsfor dynamic compressor resolution
Files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Usetypekeyword for type-only imports in TypeScript
Include.tsfile extension in local imports (e.g.,./setup.ts)
Use PascalCase for type and interface names
Files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.tspackages/action/__tests__/inputs.test.ts
packages/**/__tests__/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write tests using Vitest with
describe/test/expectin files located atpackages/*/__tests__/*.test.ts
Files:
packages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/__tests__/*.test.ts : Use shared test fixtures from `tests/fixtures.ts` with `runOneTest` helper and test suites like `commonjs`, `commoncss`, `commonhtml`
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching the `Compressor` type that receives `{ settings, content }` and returns `{ code, map? }`
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.ts.github/actions/node-minify/minify.tspackages/action/__tests__/inputs.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `resolveCompressor` and `isBuiltInCompressor` utilities from `node-minify/utils` for dynamic compressor resolution
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.ts.github/actions/node-minify/minify.tspackages/action/__tests__/inputs.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: The monorepo architecture has core packages (`core`, `utils`, `run`, `types`, `cli`, `benchmark`) with `core` depending on `utils` and `run`, and all compressor packages depending on `core`
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.ts.github/actions/node-minify/minify.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/**/__tests__/*.test.ts : Write tests using Vitest with `describe`/`test`/`expect` in files located at `packages/*/__tests__/*.test.ts`
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/__tests__/inputs.test.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities from `node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync`
Applied to files:
packages/action/__tests__/benchmark.test.tspackages/action/src/inputs.ts.github/actions/node-minify/minify.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings via `warnDeprecation` from `node-minify/utils` for deprecated packages
Applied to files:
packages/action/src/inputs.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx} : Use `type` keyword for type-only imports in TypeScript
Applied to files:
packages/action/src/inputs.ts
📚 Learning: 2026-01-09T07:41:29.716Z
Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use ES5 trailing commas in objects and arrays
Applied to files:
packages/action/src/inputs.ts
🧬 Code graph analysis (2)
packages/action/__tests__/benchmark.test.ts (2)
packages/action/src/types.ts (1)
ActionInputs(7-23)packages/benchmark/src/index.ts (1)
benchmark(16-20)
packages/action/__tests__/inputs.test.ts (1)
packages/action/src/inputs.ts (2)
parseInputs(39-115)validateCompressor(126-138)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (windows-latest, 22.x)
🔇 Additional comments (22)
.github/actions/node-minify/minify.ts (7)
5-8: LGTM!Good use of the shared
resolveCompressorutility from@node-minify/utilsfor dynamic compressor resolution. Based on learnings, this aligns with the project's recommended patterns.
10-19: LGTM!The
ActionResultinterface is well-structured with both raw and formatted values for size metrics, and appropriately uses nullable types for optional gzip data.
27-36: LGTM!The byte formatting logic is correct with proper bounds checking and edge case handling for zero bytes.
45-51: LGTM!Standard pattern for GitHub Actions output. The thrown error will be caught by the surrounding try-catch in
run().
58-61: LGTM!Simple and appropriate async wrapper for file size retrieval.
194-206: LGTM!Defense-in-depth error handling with both inner try-catch and outer promise catch. The GitHub Actions error annotation format (
::error::) is correctly used for CI visibility.
112-118: No change needed—lightningcsscorrectly does not require thetypeinput.
lightningcssis a CSS-only compressor (markedcssOnly: truein config) and its implementation does not accept or use atypeparameter. Unlikeesbuildandyui(which handle both JS and CSS), CSS-specific compressors likelightningcss,clean-css,cssnano, andcssodon't need type inference—they always process CSS. The current code is correct.packages/action/src/inputs.ts (5)
1-10: LGTM!The file header, imports, and type-only import syntax follow the coding guidelines correctly. Uses
typekeyword for the type-only import and includes the.tsextension.
11-20: LGTM!The constants are well-defined with clear deprecation messages that guide users toward modern alternatives. The ES5 trailing commas are correctly applied.
39-71: LGTM!The input parsing logic is solid:
- Type validation enforces valid values before assignment
- Compressor-type dependency is properly validated
- JSON parsing error handling avoids leaking sensitive input values in error messages
73-86: LGTM!The benchmark compressors parsing is well-implemented with deduplication, empty string filtering, and a sensible fallback to defaults. The IIFE pattern keeps the logic contained.
100-110: LGTM!The
min-reductionvalidation is thorough—handles non-numeric input, NaN, and out-of-range values (0-100). The IIFE pattern is appropriate here.packages/action/__tests__/inputs.test.ts (4)
1-19: LGTM!Correct pattern: mocks are defined before importing the system under test. This ensures the module receives the mocked dependencies. Import statements follow coding guidelines with
.tsextension.
53-120: LGTM!Tests cover the core functionality well:
- Basic input parsing
- JSON validation errors
- Type-required compressor validation
- Min-reduction bounds checking
- Benchmark compressors comma-separated parsing
122-154: LGTM!The
validateCompressortests properly verify all three scenarios: deprecated compressor warning, non-built-in compressor warning, and no warning for valid built-in compressors.
156-267: LGTM!Excellent edge case coverage:
- Invalid type values
- Valid type values ('js' and 'css')
- Deduplication of benchmark compressors
- Empty string filtering
- Fallback to defaults when all compressors are empty
- Security test ensuring raw JSON is not leaked in error messages (lines 251-266)
packages/action/__tests__/benchmark.test.ts (6)
1-12: LGTM!Correct mock-before-import pattern. Imports follow coding guidelines with
.tsextensions andtypekeyword for type-only import.
14-33: LGTM!The
baseInputsfixture correctly omits the optionalgithubTokenproperty and includesoxcin the default compressors list, aligning with the PR objective to add oxc to benchmarks.
35-62: LGTM!This test verifies the critical filtering behavior: when
typeis undefined, compressors requiring a type (likeesbuildfromTYPE_REQUIRED_COMPRESSORS) are filtered out. The assertion correctly expects["terser", "swc", "oxc"].
64-90: LGTM!Correctly verifies that when
type: "js"is provided, all compressors includingesbuildare passed to the benchmark function.
92-137: LGTM!Comprehensive test for result mapping that verifies:
originalSizeis correctly extracted- Individual compressor results are properly transformed (sizeBytes → size, reductionPercent → reduction, gzipBytes → gzipSize)
- Summary fields (
recommended,bestCompression,bestSpeed) are correctly mapped
139-155: LGTM!This test aligns with the commit message "update benchmark test to expect error on empty results". Verifies that
runBenchmarkthrows a meaningful error when no file results are returned, preventing silent failures.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/actions/node-minify/README.md:
- Around line 17-37: The migration example omits that the inputs include-gzip
and java-version were removed, which will break users; update the README
migration section to note that when replacing the old usage
(srod/node-minify/.github/actions/node-minify@main) with the new usage
(srod/node-minify@v1) the inputs include-gzip and java-version are no longer
supported, and provide guidance: explicitly list the removed inputs
(include-gzip, java-version), state that they must be removed from workflow
YAML, and add a short note or suggestions for alternatives (e.g., use a separate
gzip step or set a compatible JDK action) so users know how to migrate.
🧹 Nitpick comments (4)
packages/utils/src/getFilesizeGzippedInBytes.ts (2)
37-40: Consider static import fornode:fs/promises.
node:fsis imported statically at the top of the file (line 7), butnode:fs/promisesis dynamically imported here. For consistency and marginally better performance, consider importingreadFilestatically alongside the other Node.js imports.♻️ Suggested refactor
import { existsSync } from "node:fs"; +import { readFile } from "node:fs/promises"; import { FileOperationError } from "./error.ts"; import { isValidFile } from "./isValidFile.ts"; import { prettyBytes } from "./prettyBytes.ts";Then update the function:
const { gzipSize } = await import("gzip-size"); - const { readFile } = await import("node:fs/promises"); const content = await readFile(file); return gzipSize(content);
53-58: Useinstanceof Errorcheck instead of direct cast.The current pattern uses
error as Errorwhich is unsafe if the caught value isn't anErrorinstance. Per coding guidelines and the pattern inreadFile.ts, use a conditional check.♻️ Suggested refactor for both catch blocks
} catch (error) { throw new FileOperationError( "get gzipped size of", file, - error as Error + error instanceof Error ? error : undefined ); }Apply this change to both
getFilesizeGzippedInBytes(lines 53-58) andgetFilesizeGzippedRaw(lines 73-78).Also applies to: 73-78
.github/actions/node-minify/README.md (2)
1-16: Consider adding deprecation timeline.The deprecation notice is clear, but it doesn't specify when the legacy action will be removed or stop receiving updates. Adding a timeline (e.g., "will be removed after DATE" or "maintenance ends DATE") would help users prioritize migration.
71-82: Move java-version notice to Migration section for better visibility.This important note about
java-versionnot being supported in the new action is buried in the legacy documentation. Users following the migration guide might miss it. Consider moving this note to the Migration section (after line 37) or adding a callout near the migration example.📝 Suggested placement
After line 37 in the Migration section:
+>
⚠️ Note: Thejava-versioninput is not supported in the new bundled action. If you need Java compressors (gccoryui), useactions/setup-javabefore running the action:
+>
+>yaml +> - uses: actions/setup-java@v4 +> with: +> distribution: 'temurin' +> java-version: '17' +> - uses: srod/node-minify@v1 +> with: +> compressor: gcc +> # ... +>
+
See packages/action/README.md for full documentation.</details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a6a7a0dfc30535edcb5c163a8532b67838e50440 and 3f53cf5885f1aa26eee34cc61beb603ac6a4c971. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `.changeset/fix-gzip-stream-and-action-build.md` is excluded by `!.changeset/**` </details> <details> <summary>📒 Files selected for processing (4)</summary> * `.github/actions/node-minify/README.md` * `packages/action/package.json` * `packages/action/src/types.ts` * `packages/utils/src/getFilesizeGzippedInBytes.ts` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * packages/action/src/types.ts * packages/action/package.json </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (2)</summary> <details> <summary>**/*.{ts,tsx,js,jsx}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `**/*.{ts,tsx,js,jsx}`: Use 4 spaces for indentation > Use double quotes for strings > Always include semicolons at end of statements > Use ES5 trailing commas in objects and arrays > Use `node:` prefix for Node.js built-in imports (e.g., `node:child_process`) > Use camelCase for function and variable names > Use `try/catch` with `if (err instanceof Error)` checks for error handling > Use `Promise.all` for parallel operations where all must succeed, or `Promise.allSettled` when you need results regardless of individual failures > Limit concurrency to 5-10 for file operations and 2-4 for CPU-intensive compressions to avoid resource exhaustion > Use file operation utilities from `@node-minify/utils`: `readFile`, `writeFile`, `getFilesizeInBytes`, `getContentFromFilesAsync`, `isValidFileAsync` > Use deprecation warnings via `warnDeprecation` from `@node-minify/utils` for deprecated packages > Use `resolveCompressor` and `isBuiltInCompressor` utilities from `@node-minify/utils` for dynamic compressor resolution Files: - `packages/utils/src/getFilesizeGzippedInBytes.ts` </details> <details> <summary>**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `**/*.{ts,tsx}`: Use `type` keyword for type-only imports in TypeScript > Include `.ts` file extension in local imports (e.g., `./setup.ts`) > Use PascalCase for type and interface names Files: - `packages/utils/src/getFilesizeGzippedInBytes.ts` </details> </details><details> <summary>🧠 Learnings (5)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : UseresolveCompressorandisBuiltInCompressorutilities fromnode-minify/utilsfor dynamic compressor resolutionLearnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching theCompressortype that receives{ settings, content }and returns{ code, map? }Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: The monorepo architecture has core packages (core,utils,run,types,cli,benchmark) withcoredepending onutilsandrun, and all compressor packages depending oncore</details> <details> <summary>📚 Learning: 2026-01-09T07:41:29.716Z</summary>Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use deprecation warnings viawarnDeprecationfromnode-minify/utilsfor deprecated packages**Applied to files:** - `.github/actions/node-minify/README.md` </details> <details> <summary>📚 Learning: 2026-01-09T07:41:29.716Z</summary>Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : UseresolveCompressorandisBuiltInCompressorutilities fromnode-minify/utilsfor dynamic compressor resolution**Applied to files:** - `.github/actions/node-minify/README.md` - `packages/utils/src/getFilesizeGzippedInBytes.ts` </details> <details> <summary>📚 Learning: 2026-01-09T07:41:29.716Z</summary>Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to packages/{esbuild,google-closure-compiler,oxc,swc,terser,uglify-js,clean-css,cssnano,csso,lightningcss,html-minifier,jsonminify,no-compress}/src/index.ts : Compressor functions must export an async function matching theCompressortype that receives{ settings, content }and returns{ code, map? }**Applied to files:** - `.github/actions/node-minify/README.md` </details> <details> <summary>📚 Learning: 2026-01-09T07:41:29.716Z</summary>Learnt from: CR
Repo: srod/node-minify PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-09T07:41:29.716Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use file operation utilities fromnode-minify/utils:readFile,writeFile,getFilesizeInBytes,getContentFromFilesAsync,isValidFileAsync**Applied to files:** - `.github/actions/node-minify/README.md` - `packages/utils/src/getFilesizeGzippedInBytes.ts` </details> </details><details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>packages/utils/src/getFilesizeGzippedInBytes.ts (4)</summary><blockquote> <details> <summary>packages/utils/src/error.ts (1)</summary> * `FileOperationError` (11-25) </details> <details> <summary>packages/utils/src/index.ts (4)</summary> * `isValidFile` (46-46) * `readFile` (49-49) * `getFilesizeGzippedInBytes` (40-40) * `getFilesizeGzippedRaw` (41-41) </details> <details> <summary>packages/utils/src/isValidFile.ts (1)</summary> * `isValidFile` (21-27) </details> <details> <summary>packages/utils/src/readFile.ts (1)</summary> * `readFile` (36-46) </details> </blockquote></details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)</summary> * GitHub Check: cubic · AI code reviewer * GitHub Check: test (ubuntu-latest, 20.x) * GitHub Check: test (ubuntu-latest, 24.x) * GitHub Check: test (windows-latest, 24.x) * GitHub Check: test (windows-latest, 20.x) * GitHub Check: test (windows-latest, 22.x) * GitHub Check: test (ubuntu-latest, 22.x) </details> <details> <summary>🔇 Additional comments (7)</summary><blockquote> <details> <summary>packages/utils/src/getFilesizeGzippedInBytes.ts (2)</summary><blockquote> `7-10`: **LGTM!** Imports correctly use the `node:` prefix for Node.js built-ins and `.ts` extensions for local imports as per coding guidelines. --- `62-80`: **LGTM on the new `getFilesizeGzippedRaw` API.** Clean addition that complements `getFilesizeGzippedInBytes` by providing the raw numeric value. The JSDoc with example is helpful for consumers. </blockquote></details> <details> <summary>.github/actions/node-minify/README.md (5)</summary><blockquote> `39-39`: **LGTM!** The relative path to the new action's README is correct. --- `41-56`: **LGTM!** The legacy documentation section is clearly marked and the basic example correctly demonstrates the deprecated action usage. --- `58-69`: **LGTM!** The inputs table clearly documents the legacy composite action's parameters. --- `97-99`: **LGTM!** Standard license declaration is clear and appropriate. --- `84-95`: The outputs table is correctly placed. It documents the legacy composite action's outputs, which belong in the "Legacy Documentation" section. Verification confirms all 8 outputs in the table match the legacy action's outputs exactly. However, the new bundled action exports 5 additional outputs (`report-json`, `benchmark-winner`, `best-compression`, `best-speed`, `benchmark-json`) that are not documented anywhere in the README. Consider adding a section documenting these new action outputs to help users understand what the migration provides. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@greptile |
1 similar comment
|
@greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2760 +/- ##
===========================================
+ Coverage 99.54% 99.58% +0.03%
===========================================
Files 58 62 +4
Lines 1105 1199 +94
Branches 335 367 +32
===========================================
+ Hits 1100 1194 +94
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
This PR introduces a proper GitHub Action package at
packages/actionand configures the repository for GitHub Marketplace publication.Key Changes
packages/action(Node.js 20 runtime) replaces the legacy composite action.action.ymlat root pointing to the bundled action, enablinguses: srod/node-minify@v1.@node-minify/core,@node-minify/utilsand action logic into a single file (packages/action/dist/index.js), improving startup time and reliability.oxcto default benchmark compressors.docs/src/content/docs/github-action.md.release-action.yml: Automates publishing to GitHub Marketplace.test-action.yml: Updated to test the bundled action with real minification tasks.Features
Migration
Users of the old action should migrate:
Summary by cubic
Adds an official Node.js 20 GitHub Action package for node-minify and enables Marketplace usage via uses: srod/node-minify@v1. Deprecates the old composite action and adds richer reporting, benchmarking, threshold enforcement, and release automation.
New Features
Migration
Written for commit fad512f. Summary will update on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.