feat: migrate from JSON/bun to pnpm/SQLite/vitest - #1
Conversation
- Migrate storage from JSON to SQLite (better-sqlite3) - Switch to pnpm, vitest, tsx (drop bun) - Fix table display, delete cascade, add error handling - Add 28 tests, CI workflow, Renovate config
|
Warning Review limit reached
More reviews will be available in 48 minutes and 15 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR migrates the subsc-cli project from Bun to Node.js + pnpm, replacing JSON-based subscription storage with SQLite via better-sqlite3. The changes include a new GitHub Actions CI pipeline, updated package configuration, a complete storage layer rewrite, and CLI/display updates to integrate the new API. ChangesBun to Node.js + SQLite Migration
Sequence DiagramsequenceDiagram
participant CLI as CLI add command
participant Inquirer as Inquirer prompts
participant WriteAPI as writeSubscription()
participant DB as SQLite database
participant Display as spreadSubscription()
CLI->>Inquirer: collect name/price/currency/cycle/tags
Inquirer->>WriteAPI: provide AddSharedArgs
WriteAPI->>DB: INSERT subscription
WriteAPI->>DB: INSERT OR IGNORE tags
WriteAPI->>DB: INSERT subscription_tags relations
WriteAPI->>Display: subscription written
Display->>Display: format table with ¥/$ symbols
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 unit tests (beta)
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 |
pnpm v11 requires Node.js v22.13+, so Node.js 20 is no longer supported. Removing it from the CI matrix to fix CI failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
subsc-cli/src/table.test.ts (1)
25-35: ⚡ Quick winType
makeSubasSharedArgsto keep fixtures contract-safe under strict TS.Using
Record<string, unknown>weakens compile-time checks for overridden fields in tests.Suggested fix
+import type { SharedArgs } from "./basefs" -function makeSub(overrides: Record<string, unknown> = {}) { +function makeSub(overrides: Partial<SharedArgs> = {}): SharedArgs { return { id: 1, name: "Test Service", price: 1000, currency: "JPY", cycle: "monthly", tags: [], ...overrides, - }; + } }🤖 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 `@subsc-cli/src/table.test.ts` around lines 25 - 35, The fixture factory makeSub should use the SharedArgs type instead of Record<string, unknown>: change its signature to makeSub(overrides: Partial<SharedArgs> = {}): SharedArgs (and import SharedArgs if not already imported), update the returned object type to satisfy SharedArgs (keep the existing default fields and spread overrides) so TypeScript validates override keys and types under strict mode.subsc-cli/src/index.ts (1)
3-3: ⚡ Quick winShared style drift: import semicolons violate the repo’s ESM import/export rule. Apply the same no-semicolon import style consistently in both changed sites.
subsc-cli/src/index.ts#L3-L3: remove the trailing semicolon from the changed import.subsc-cli/src/table.test.ts#L1-L3: remove trailing semicolons from all changed imports.As per coding guidelines: "
subsc-cli/**/*.{ts,tsx,js,jsx}... Use no semicolons in imports/exports (ESM style)".🤖 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 `@subsc-cli/src/index.ts` at line 3, Remove the trailing semicolons from the ESM import lines to match the repo style: in subsc-cli/src/index.ts (lines 3-3) change the import "import { input, select } from \"`@inquirer/prompts`\";" to omit the trailing semicolon, and in subsc-cli/src/table.test.ts (lines 1-3) remove trailing semicolons from the changed import statements on those lines so all imports use the no-semicolon ESM style.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 6-11: The workflow's path filter currently limits runs to
"subsc-cli/**" (the push and pull_request "paths" keys), so edits to the
workflow itself won't trigger CI; update the push and pull_request "paths"
configuration (or remove path filtering entirely) to include the workflow files
(add ".github/workflows/**") or delete the restrictive "paths" entries so
changes to the CI config will run the workflow; ensure changes are applied to
both the push "paths" block and the pull_request "paths" block.
- Around line 26-36: Pin the three mutable action references and disable
credential persistence: replace actions/checkout@v4, pnpm/action-setup@v4 and
actions/setup-node@v4 with their respective full commit SHAs (use the commit SHA
for the desired release tags) and add persist-credentials: false to the
actions/checkout step to prevent credential leakage; additionally, update the
workflow trigger paths (the push/pull_request paths filter) to include
.github/workflows/** (or remove the restrictive paths filter) so changes to
workflow files will trigger CI.
In `@subsc-cli/src/basefs.ts`:
- Around line 173-189: Deduplicate the incoming tags before building the SQL so
duplicate filter args don't incorrectly increase the required COUNT; replace
uses of the local tags array with a deduped array (e.g., uniqueTags =
Array.from(new Set(tags))) and check uniqueTags.length === 0, build placeholders
from uniqueTags, and call .all(...uniqueTags, uniqueTags.length) and use
uniqueTags.length in the HAVING COUNT(...) parameter so the COUNT(DISTINCT ...)
comparison matches the number of distinct requested tags.
- Around line 30-58: Enable SQLite foreign key enforcement and add the missing
test assertion: in subsc-cli/src/basefs.ts (lines 30-58) after creating _db and
setting pragma journal_mode, execute PRAGMA foreign_keys = ON on the Database
instance (ensure _db.pragma or _db.exec is used to enable foreign keys so the
subscription_tags FOREIGN KEY ... ON DELETE CASCADE takes effect); in
subsc-cli/src/basefs.test.ts (lines 5-6) where the in-memory testDb is created,
also enable PRAGMA foreign_keys = ON on that test DB instance; and in
subsc-cli/src/basefs.test.ts (lines 161-176) extend the deletion test to assert
that subscription_tags rows for the deleted subscription are removed (e.g.,
query subscription_tags for the deleted subscription_id and assert the result
set is empty) while leaving the existing getSubscriptions() assertion intact.
- Around line 113-147: The writeSubscription() flow currently inserts via
insertSub then inserts tags and subscription_tags (using insertTag, getTagId,
insertRel) without a transaction and can leave a partially persisted
subscription; wrap the entire sequence (insertSub + tag deduping + inserting
relations) in a single DB transaction so either everything commits or nothing
does, and before inserting relation rows dedupe the data.tags array (e.g.,
unique set) to avoid redundant insertTag/getTagId/insertRel calls and potential
uniqueness constraint failures on subscription_tags; ensure you still use INSERT
OR IGNORE for tags and then query tag ids with getTagId inside the same
transaction so relations use the correct ids.
In `@subsc-cli/src/index.ts`:
- Around line 26-30: The validate function for the amount input currently treats
empty string as 0 because Number("") === 0; update the validator in
subsc-cli/src/index.ts (the validate callback) to first reject blank input
(e.g., if value.trim() === '' return an error) before converting to Number, and
change the error message from "Please enter a valid positive number" to reflect
that zero is allowed (for example "Please enter a valid non-negative number");
keep the existing numeric check (isNaN/Number(value) < 0) after the blank check.
---
Nitpick comments:
In `@subsc-cli/src/index.ts`:
- Line 3: Remove the trailing semicolons from the ESM import lines to match the
repo style: in subsc-cli/src/index.ts (lines 3-3) change the import "import {
input, select } from \"`@inquirer/prompts`\";" to omit the trailing semicolon, and
in subsc-cli/src/table.test.ts (lines 1-3) remove trailing semicolons from the
changed import statements on those lines so all imports use the no-semicolon ESM
style.
In `@subsc-cli/src/table.test.ts`:
- Around line 25-35: The fixture factory makeSub should use the SharedArgs type
instead of Record<string, unknown>: change its signature to makeSub(overrides:
Partial<SharedArgs> = {}): SharedArgs (and import SharedArgs if not already
imported), update the returned object type to satisfy SharedArgs (keep the
existing default fields and spread overrides) so TypeScript validates override
keys and types under strict mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f92bb702-df19-49ea-9451-1515eb117493
⛔ Files ignored due to path filters (2)
subsc-cli/bun.lockis excluded by!**/*.locksubsc-cli/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.github/workflows/ci.ymlrenovate.jsonsubsc-cli/.gitignoresubsc-cli/.npmignoresubsc-cli/CLAUDE.mdsubsc-cli/package.jsonsubsc-cli/pnpm-workspace.yamlsubsc-cli/src/basefs.test.tssubsc-cli/src/basefs.tssubsc-cli/src/calc.tssubsc-cli/src/index.tssubsc-cli/src/table.test.tssubsc-cli/src/table.tssubsc-cli/tsconfig.json
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 11 | ||
|
|
||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| cache: "pnpm" | ||
| cache-dependency-path: "subsc-cli/pnpm-lock.yaml" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "=== File: $FILE ==="
if [ ! -f "$FILE" ]; then
echo "Missing $FILE" >&2
exit 1
fi
wc -l "$FILE"
nl -ba "$FILE" | sed -n '1,160p'
echo
echo "=== Search for checkout persist-credentials ==="
rg -n "persist-credentials" "$FILE" || true
echo
echo "=== Search for pinning to full commit SHAs for actions ==="
rg -n "uses:\s*(actions/|pnpm/|.*`@v`)" "$FILE" || true
echo
echo "=== Show trigger block ==="
# Extract on: block roughly (from first 'on:' to next top-level key)
awk '
/^on:/ {in_on=1}
in_on {print}
in_on && /^[a-zA-Z0-9_ -]+:/ && $1!="on:" {exit}
' "$FILE" | sed -n '1,120p' || trueRepository: nazozokc/subsc-cli
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "=== File: $FILE ==="
[ -f "$FILE" ] || { echo "Missing $FILE" >&2; exit 1; }
echo "--- Line count ---"
wc -l "$FILE"
echo
echo "--- File preview (1-160) with line numbers via awk ---"
awk 'NR<=160{printf "%4d: %s\n", NR, $0}' "$FILE"
echo
echo "=== Search for checkout persist-credentials ==="
rg -n "persist-credentials" "$FILE" || true
echo
echo "=== Search for unpinned action refs (major-only like `@v4`) ==="
rg -n "uses:\s*[^#\n]+@v[0-9]+(\b|$)" "$FILE" || true
echo
echo "=== Search for action refs pinned to full SHAs (40 hex) ==="
rg -n "uses:\s*[^#\n]+@[0-9a-f]{40}\b" "$FILE" || true
echo
echo "--- Show trigger block around `on:` ---"
awk '
/^on:/ {print; in_on=1; next}
in_on {
# stop when a top-level key starts (no indentation) that's not part of on:
if ($0 ~ /^[a-zA-Z0-9_ -]+:/ && $0 !~ /^on:/ && $0 !~ /^ /) {exit}
print
}
' "$FILE" | sed -n '1,160p'Repository: nazozokc/subsc-cli
Length of output: 1913
🌐 Web query:
GitHub Actions actions/checkout persist-credentials default value
💡 Result:
The default value for the persist-credentials input in the GitHub Actions actions/checkout action is true [1][2]. When set to true (the default), the action configures the authentication token or SSH key used for the checkout in the local git configuration [1][2]. This allows your workflow scripts to perform authenticated git operations (such as git fetch or git push) without requiring additional manual configuration [3][4]. The action automatically removes these credentials from the local git configuration during the post-job cleanup phase [3][4]. If you wish to disable this behavior, you can explicitly set persist-credentials to false in your workflow file [3][5].
Citations:
- 1: https://github.com/actions/checkout/blob/72f2cec99f417b1a1c5e2e88945068983b7965f9/action.yml
- 2: https://github.com/actions/checkout/blob/v4/action.yml
- 3: https://github.com/actions/checkout/?tab=readme-ov-file
- 4: https://github.com/actions/checkout
- 5: https://github.com/actions/checkout/blob/v4/README.md
🌐 Web query:
GitHub Actions workflow pathsfilter trigger when only.github/workflows/** changes
💡 Result:
Yes, you can configure a GitHub Actions workflow to trigger when changes are made to files within the.github/workflows/ directory by using the paths filter in your workflow configuration [1][2]. To trigger a workflow only when files in the.github/workflows/ directory change, use the following syntax in your workflow YAML file [1]: on: push: paths: - '.github/workflows/' This configuration ensures that the workflow will run if any file within the.github/workflows/ directory is modified in a push event [1]. You can use glob patterns, such as the double asterisk, to match all files recursively within that directory [3][4]. Key considerations: 1. Trigger Event: The paths filter works with the push and pull_request events [5][3]. 2. Path Filters and Other Triggers: If you have other triggers defined in the same workflow (such as schedule or workflow_dispatch), the paths filter only applies to the push or pull_request events it is associated with [1]. 3. Exclusions: If you need to trigger on changes to the workflow directory but ignore specific files within it, you cannot combine paths and paths-ignore for the same event [3]. Instead, you can use the! character within the paths filter to exclude specific files [5][3]. If you need more granular control—such as triggering based on specific workflow files while excluding others, or applying logic at the job or step level—you may consider using a third-party action like dorny/paths-filter, which allows for conditional execution based on file changes [6][7].
Citations:
- 1: https://stackoverflow.com/questions/73646221/trigger-github-actions-workflow-when-some-workflows-themselves-change
- 2: https://docs.github.com/actions/using-workflows/triggering-a-workflow
- 3: https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions
- 4: https://github.blog/changelog/2019-09-30-github-actions-event-filtering-updates/
- 5: http://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow
- 6: https://github.com/dorny/paths-filter
- 7: https://nedbatchelder.com/blog/202505/filtering_github_actions_by_changed_files
Pin GitHub Actions to commit SHAs and disable checkout credential persistence.
- In
.github/workflows/ci.yml,actions/checkout@v4/pnpm/action-setup@v4/actions/setup-node@v4are only tagged (mutable); pin each to a full commit SHA. - Add
persist-credentials: falseto theactions/checkoutstep (current default istrue). - CI won’t run for changes limited to
.github/workflows/**becauseon: push/pull_requestpathsis restricted tosubsc-cli/**.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 26-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 26-26: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 32-32: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 26 - 36, Pin the three mutable action
references and disable credential persistence: replace actions/checkout@v4,
pnpm/action-setup@v4 and actions/setup-node@v4 with their respective full commit
SHAs (use the commit SHA for the desired release tags) and add
persist-credentials: false to the actions/checkout step to prevent credential
leakage; additionally, update the workflow trigger paths (the push/pull_request
paths filter) to include .github/workflows/** (or remove the restrictive paths
filter) so changes to workflow files will trigger CI.
Source: Linters/SAST tools
- Enable PRAGMA foreign_keys = ON in production and test DB - Wrap writeSubscription() in a transaction for atomicity - Deduplicate tags in tagsSubscription() to prevent false negatives - Add .github/workflows/** to CI path filter - Remove semicolons from ESM imports per repo style - Fix amount validate to reject blank input and correct error message - Use Partial<SharedArgs> in makeSub fixture for type safety - Verify subscription_tags cascade in deletion test
1 participant
Summary
Complete rewrite of the storage layer and toolchain migration.
Changes
Storage: JSON file → SQLite (better-sqlite3) with proper schema (subscriptions, tags, subscription_tags with ON DELETE CASCADE)
Toolchain:
Bug fixes:
New files: 28 tests (basefs CRUD + table display), CI workflow (Node 20/22/23), Renovate config
Refactoring: lazy DB singleton, env var override (
SUBSC_CLI_DB_DIR), in-memory DB injection for testsVerification
pnpm build— tsdown builds successfullypnpm test— 28 tests passpnpm start --help— CLI starts correctlySummary by CodeRabbit
New Features
Chores