fix(ccusage): require Node 22 for Node runtime#1006
Conversation
Guard the published CLI wrapper before it falls back to the Node entrypoint. When Bun is available, the wrapper still delegates to the Bun bundle without checking the Bun version. When Bun auto-run is disabled or Bun is unavailable, Node.js 22 and older now exit with a clear runtime error instead of starting the Node bundle. Align the ccusage package engines field with that runtime requirement.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR refactors the CLI to enforce Node.js 22+ with Bun fallback. A new ChangesNode.js Version Gating and Runtime Selection
Sequence Diagram(s)sequenceDiagram
participant runCli
participant resolveCliRuntime
participant BunExecutable
participant NodeRuntime
participant ChildProcess
runCli->>resolveCliRuntime: determine runtime (Bun or Node) or error
resolveCliRuntime-->>runCli: {command,args} or {errorMessage}
alt errorMessage returned
runCli->>runCli: write errorMessage to stderr and exit 1
else Bun selected
runCli->>BunExecutable: spawn `bun main.bun.js` (inherited stdio)
BunExecutable-->>ChildProcess: process runs Bun entrypoint
else Node selected
runCli->>NodeRuntime: spawn `process.execPath main.node.js` (inherited stdio)
NodeRuntime-->>ChildProcess: process runs Node entrypoint
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/ccusage/src/cli.ts[baseline-browser-mapping] The data in this module is over two months old. To ensure accurate Baseline data, please update: Oops! Something went wrong! :( ESLint: 9.35.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'eslint-plugin-format' imported from /node_modules/.pnpm/@antfu+eslint-config@4.19.0_@vue+compiler-sfc@3.5.30_eslint@9.35.0_typescript@5.9.2_vit_670a2c5c75d4275eabd7bc195a173ee6/node_modules/@antfu/eslint-config/dist/index.js 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 |
Lower the Node runtime guard from Node.js 23 to Node.js 22 so it matches the package engine and intended support floor. Bun delegation remains unchanged. The guard only rejects Node.js 21 and older when the wrapper falls back to the Node bundle.
@ccusage/amp
ccusage
@ccusage/codex
@ccusage/opencode
@ccusage/pi
commit: |
ryoppippi
changed the title
ccusage performance comparisonThis compares the PR build against the base branch build on the same CI runner. Committed fixture performanceCommitted small fixture for stable PR-to-PR feedback and output-shape regressions. Fixture:
Large real-world-shaped fixture performanceGenerated fixture around 1 GiB shaped from aggregate local Claude-log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixture. Fixture:
Package size
Lower medians and smaller packed package sizes are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ⛔ Deployment terminated View logs |
ccusage-guide | d97c612 | May 16 2026, 07:01 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/ccusage/src/cli.ts (1)
158-189: ⚡ Quick winAdd test coverage for Node.js 23+ success case.
The current tests validate that Node 22 is rejected and that Bun is used when available. Consider adding a test case for the primary scenario this PR enables: Node.js 23+ should be accepted when Bun is unavailable.
🧪 Suggested test case
it('rejects Node.js 22 when Bun is unavailable', () => { expect( resolveCliRuntime({ argv: ['daily'], distDir: '/app/dist', findBunPath: () => undefined, isRunningInBun: false, nodeVersion: 'v22.13.1', processExecPath: '/usr/bin/node', }), ).toEqual({ errorMessage: 'ccusage requires Bun or Node.js >=23.0.0. Current Node.js: v22.13.1\n', }); }); + + it('accepts Node.js 23 when Bun is unavailable', () => { + expect( + resolveCliRuntime({ + argv: ['daily'], + distDir: '/app/dist', + findBunPath: () => undefined, + isRunningInBun: false, + nodeVersion: 'v23.0.0', + processExecPath: '/usr/bin/node', + }), + ).toEqual({ + args: ['/app/dist/main.node.js', 'daily'], + command: '/usr/bin/node', + }); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ccusage/src/cli.ts` around lines 158 - 189, Add a test that asserts resolveCliRuntime accepts Node.js >= v23 when Bun is unavailable: call resolveCliRuntime with isRunningInBun: false, findBunPath: () => undefined, nodeVersion: 'v23.0.0' (or higher), argv and distDir as in existing tests and expect a successful result (command set to processExecPath and args pointing to distDir/main.js with the provided argv) to verify the main Node.js path is chosen; locate the tests around resolveCliRuntime in the existing describe block to add this case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/ccusage/src/cli.ts`:
- Around line 158-189: Add a test that asserts resolveCliRuntime accepts Node.js
>= v23 when Bun is unavailable: call resolveCliRuntime with isRunningInBun:
false, findBunPath: () => undefined, nodeVersion: 'v23.0.0' (or higher), argv
and distDir as in existing tests and expect a successful result (command set to
processExecPath and args pointing to distDir/main.js with the provided argv) to
verify the main Node.js path is chosen; locate the tests around
resolveCliRuntime in the existing describe block to add this case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1301b7c-3e6b-46cd-91bd-f43da14251d6
📒 Files selected for processing (2)
apps/ccusage/package.jsonapps/ccusage/src/cli.ts
Use the Bun global object to recognise when the wrapper is already running under Bun. That covers bun --bun/-b execution without relying on process.versions.bun. When Bun is already active, runtime resolution returns the Bun entrypoint directly and does not scan PATH or evaluate the Node version guard.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ccusage/src/cli.ts`:
- Around line 109-113: The conditional that decides the Bun runtime path
currently checks bunAutoRunValue against CCUSAGE_BUN_AUTO_RUN_DISABLED_VALUE
before isBunRuntime, causing an active Bun process to be gated by the auto-run
env; update the ternary logic so isBunRuntime is evaluated first (return
processExecPath when isBunRuntime is true), otherwise apply the bunAutoRunValue
check and call findBunPath() only when auto-run is enabled; make the same
reorder for the identical block that appears around lines 121-124, referencing
the same symbols isBunRuntime, CCUSAGE_BUN_AUTO_RUN_DISABLED_VALUE,
processExecPath, and findBunPath to locate and fix both occurrences.
- Around line 11-18: Replace the custom union type CliRuntime with the byethrow
Result type: import the Result type from byethrow and define CliRuntime as
Result<{ args: string[]; command: string }, string> (or Result<SuccessShape,
ErrorMessageType>), then update all places that construct or pattern-match on
CliRuntime (including the other occurrences referenced) to use byethrow
conventions (e.g., Result.ok / Result.err or the repo’s standard helpers)
instead of the { errorMessage } | { command,args } union.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| type CliRuntime = | ||
| | { | ||
| args: string[]; | ||
| command: string; | ||
| } | ||
| | { | ||
| errorMessage: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace custom Result union with byethrow Result type
This file introduces manual { errorMessage } | { command,args } Result modeling. The repo rule for apps/ccusage/src/**/*.ts requires byethrow for Result-based error handling; please align this branch to byethrow to keep error flow consistent.
As per coding guidelines, “Use byethrow for Result-based error handling”.
Also applies to: 121-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ccusage/src/cli.ts` around lines 11 - 18, Replace the custom union type
CliRuntime with the byethrow Result type: import the Result type from byethrow
and define CliRuntime as Result<{ args: string[]; command: string }, string> (or
Result<SuccessShape, ErrorMessageType>), then update all places that construct
or pattern-match on CliRuntime (including the other occurrences referenced) to
use byethrow conventions (e.g., Result.ok / Result.err or the repo’s standard
helpers) instead of the { errorMessage } | { command,args } union.
Short-circuit the CLI wrapper when it is already running under Bun. Instead of spawning another Bun process, the wrapper dynamically imports the Bun entrypoint and returns before PATH lookup or Node runtime checks. Node-launched wrapper behaviour is unchanged: it still scans PATH for Bun when auto-run is enabled, then falls back to the Node entrypoint with a Node.js 22 minimum.
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Refactor
Tests