feat(cli): polish CLI output, dev checks, and update notifications#116
Merged
feat(cli): polish CLI output, dev checks, and update notifications#116
Conversation
Export schema from paykitjs/database so consumers can reference it without reaching into internal package paths. Add drizzle config and db:studio script to demo app.
…shared utils BREAKING: `paykitjs check` is removed. Use `paykitjs status` instead. Pass `--throw` to exit with code 1 on issues (for CI). - Add loading spinner during async work - Remove command name echo (p.intro) - Parallelize DB and Stripe checks in status - Lazy-load command modules and heavy deps for instant spinner - Skip runtime dev sync check during CLI via PAYKIT_CLI env var - Fire telemetry early, don't await flush on success (fixes hang) - Extract shared CLI utils (loadCliDeps, checkDatabase, checkStripe, etc.) - Show full package manager command (e.g. pnpm dlx paykitjs push) - Context-aware outro messages (migrations vs products vs both) - Surface Stripe and DB errors with clear messages - Add getRunCommand helper to detect.ts - Rename check command to status, add --throw flag - Fix e2e test fixture Pool reference and suppress runtime sync check
…migrations On dev startup, PayKit now checks database connectivity, pending migrations, and product sync status. Errors surface as clear log messages instead of cryptic query failures.
Single spinner for all loading, consistent section headers with p.log.info, confirmation prompt at the end showing full context, and sync result in outro.
Remove ~/.paykit/ config directory, telemetry subcommand, and first-run notice. Telemetry is now disabled via PAYKIT_TELEMETRY_DISABLED=1 or DO_NOT_TRACK=1 environment variables.
Checks that all @paykitjs/* packages are on the same version at dev startup. Also simplifies dev check logging to use console.warn with picocolors styling, removes separate DB connectivity check, and uses globalThis to deduplicate checks across module re-evaluations.
Checks npm registry for latest version in parallel with command execution and shows a notification after the outro if an update is available. Uses detected package manager for the install command.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors CLI to dynamic command loading and shared DI utilities; replaces plan-based syncs with product-based flows (status/push); removes telemetry subcommands in favor of env-driven control; adds update-check and dependency tools; exposes database schema; adds Drizzle config and Suspense fallbacks; introduces multiple utilities, dev checks, and test config tweaks. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(240,248,255,0.5)
participant CLI as CLI Entry
participant Loader as loadCliDeps
participant DB as PostgreSQL
participant Stripe as Stripe API
participant Sync as Product Sync
participant User as Console
end
CLI->>CLI: determine command (status / push / init)
CLI->>Loader: loadCliDeps()
Loader-->>CLI: CliDeps (Pool, StripeSdk, ctx creators, helpers)
alt status flow
CLI->>Loader: createPool(connection)
Loader->>DB: open pool / ping
par concurrent checks
Loader->>DB: query pending migrations
Loader->>Stripe: fetch account & webhooks
end
CLI->>Loader: loadProductDiffs(config)
Loader->>Sync: dryRunSyncProducts(ctx)
Sync-->>Loader: diffs
Loader-->>CLI: formatted status blocks
CLI->>User: print status output
opt --throw
CLI->>CLI: exit(1) if issues
end
else push flow
CLI->>Loader: createPool & checkStripe
CLI->>User: prompt confirmation
User-->>CLI: confirm
CLI->>Loader: migrateDatabase()
Loader->>DB: run migrations
CLI->>Loader: syncProducts(ctx)
Sync-->>Loader: complete
CLI->>User: print "products synced"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing/content/docs/concepts/cli.mdx (1)
43-57:⚠️ Potential issue | 🟠 MajorRemove stale
paykitjs telemetrydocs after command removal.Line 47 onward still documents
paykitjs telemetry, but this PR removes telemetry subcommands. This will direct users to a non-existent command.📘 Suggested docs fix
-## `paykitjs telemetry` - -<PackageRun command="paykitjs telemetry status" /> - -Manages anonymous usage telemetry. PayKit collects no personally identifiable information. Subcommands: - -- `enable` - opt in to telemetry -- `disable` - opt out -- `status` - show the current setting - -You can also disable telemetry permanently by setting the `PAYKIT_TELEMETRY_DISABLED` or `DO_NOT_TRACK` environment variables. Either one takes precedence over the local setting. +## Telemetry + +Telemetry can be disabled via environment variables: + +- `PAYKIT_TELEMETRY_DISABLED=1` +- `DO_NOT_TRACK=1`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@landing/content/docs/concepts/cli.mdx` around lines 43 - 57, Remove the entire `paykitjs telemetry` subsection (including the `<PackageRun command="paykitjs telemetry status" />` example), the bullet list of telemetry subcommands (`enable`, `disable`, `status`), and the paragraph about `PAYKIT_TELEMETRY_DISABLED` / `DO_NOT_TRACK` from the CLI docs so no references to the removed `paykitjs telemetry` command remain; if you want to keep users informed, replace that block with a single-line note stating telemetry subcommands were removed and point to the relevant changelog or migration doc.
🧹 Nitpick comments (3)
e2e/cli/setup.ts (1)
9-10: ScopePAYKIT_CLIenv mutation to test lifecycle.Setting this at module load can leak across test suites. Prefer setting/restoring it in fixture setup/cleanup for better isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/cli/setup.ts` around lines 9 - 10, The file currently sets process.env.PAYKIT_CLI = "1" at module load which can leak across tests; change this to set and restore the environment variable inside your test lifecycle (e.g., in your test fixture or beforeEach/afterEach hooks): capture the original value of process.env.PAYKIT_CLI, assign "1" at setup, and restore the original value in cleanup so the mutation is scoped to the test. Update any related fixture file or setup/teardown logic that uses process.env.PAYKIT_CLI to perform this save/restore rather than assigning at module top-level.packages/paykit/src/utilities/dependencies/paykit-package-list.ts (1)
1-1: Make the package list immutable to reduce drift risk.Consider exporting this as
as const(readonly literals) and reusing it in install/check flows to avoid divergence between multiple hardcoded package arrays.♻️ Suggested refactor
-export const PAYKIT_PACKAGE_LIST = ["paykitjs", "@paykitjs/stripe", "@paykitjs/dash"]; +export const PAYKIT_PACKAGE_LIST = ["paykitjs", "@paykitjs/stripe", "@paykitjs/dash"] as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/utilities/dependencies/paykit-package-list.ts` at line 1, The package array PAYKIT_PACKAGE_LIST should be made immutable to prevent drift; change its export to use readonly literal tuples (e.g., export const PAYKIT_PACKAGE_LIST = [...] as const) and update any install/check code to consume that constant (or derive a string[] via Array.from or typeof PAYKIT_PACKAGE_LIST[number] where needed) so all flows reuse the single immutable source of truth.packages/paykit/src/cli/utils/shared.ts (1)
91-94: Precompute plan lookup to avoid repeated linear scans
plans.find(...)insidediffs.map(...)does O(n*m) work. Build a map once for better scalability when there are many plans/diffs.Proposed refactor
export function formatProductDiffs( diffs: ProductDiff[], plans: readonly NormalizedPlan[], deps: Pick<CliDeps, "formatPlanLine" | "formatPrice">, ): string[] { + const plansById = new Map(plans.map((pl) => [pl.id, pl])); return diffs.map((diff) => { - const plan = plans.find((pl) => pl.id === diff.id); + const plan = plansById.get(diff.id); const price = plan ? deps.formatPrice(plan.priceAmount ?? 0, plan.priceInterval) : "$0"; return deps.formatPlanLine(diff.action, diff.id, price); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/cli/utils/shared.ts` around lines 91 - 94, The current diffs.map callback does a linear search with plans.find for each diff causing O(n*m) complexity; instead, precompute a lookup (e.g., const plansById = new Map(plans.map(p => [p.id, p])) ) before the map and then inside the mapping use plansById.get(diff.id) to get the plan; update the price calculation to use the found plan (plan?.priceAmount ?? 0 and plan?.priceInterval) and keep the deps.formatPlanLine(diff.action, diff.id, price) call unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/demo/drizzle.config.ts`:
- Around line 7-9: Replace the non-null assertion on process.env.DATABASE_URL in
dbCredentials with an explicit runtime check that throws a clear, descriptive
error if DATABASE_URL is missing; update the drizzle config's dbCredentials
block to read the env var into a local constant (e.g., databaseUrl) and validate
it, then use that variable instead of process.env.DATABASE_URL!, so any missing
configuration surfaces as a helpful error at startup.
In `@docs/superpowers/plans/2026-04-09-docs-content.md`:
- Around line 564-565: Remove the stale "paykitjs telemetry" CLI documentation
entry: delete or replace the bullet that documents the `paykitjs telemetry`
command (and its subcommands `enable`, `disable`, `status`) so docs no longer
reference the removed telemetry command; ensure only the `paykitjs status` entry
(and any other current commands) remain and that references to env vars
`PAYKIT_TELEMETRY_DISABLED` and `DO_NOT_TRACK` are removed or moved only if
still relevant elsewhere.
In `@packages/paykit/package.json`:
- Around line 34-35: The package exports include an entry named "./database"
that points to built artifacts that are never emitted because tsdown.config.ts
lacks an entry for src/database/schema.ts; add a build entry in tsdown.config.ts
for src/database/schema.ts so the dist/database/schema.js and
dist/database/schema.d.ts are generated during the build and satisfy the
"./database" export (ensure the entry uses the same source path and will emit to
dist/database/schema.js/.d.ts).
In `@packages/paykit/src/cli/commands/push.ts`:
- Around line 33-49: Before treating a failure as "pending migrations", perform
the explicit DB connectivity check used in the status flow and bail early on
connection errors: call the same DB-check helper from the status command (the
explicit connectivity check) before invoking
deps.getPendingMigrationCount(database); if the check reports a
connectivity/error condition, stop the spinner, log the DB error and exit
instead of proceeding to deps.migrateDatabase(database). Ensure you keep the
existing references to deps.getPendingMigrationCount and deps.migrateDatabase
but gate them behind the status-style DB connectivity check.
- Around line 27-35: The code dereferences config.options.provider.secretKey
without checking provider exists, causing a TypeError for configs missing
provider; update the push command flow in push.ts to validate that
config.options.provider is present before calling checkStripe (or reading
secretKey) — mirror the same provider-existence check used in the status path
and, if absent, surface the same CLI-facing “No provider configured” error
instead of proceeding to checkStripe; adjust the call site where
checkStripe(deps, config.options.provider.secretKey) is invoked so it only runs
after the provider validation.
In `@packages/paykit/src/cli/index.ts`:
- Around line 11-37: The lazy imports and program.addCommand calls
(statusCommand, initCommand, pushCommand, and Promise.all block) currently run
before the try block so import failures bypass the CLI's friendly
error/telemetry/exit handling; move the entire command loading and registration
logic (the switch on commandName and the default Promise.all import +
program.addCommand calls) inside the existing try or into an awaited helper
invoked from the try so any import errors are caught and handled by the existing
error/telemetry/exit path.
In `@packages/paykit/src/cli/utils/detect.ts`:
- Around line 34-39: getRunCommand currently returns `yarn dlx` for
PackageManager "yarn", which breaks for Yarn Classic (v1); update getRunCommand
to detect Yarn version and fall back to `npx ${script}` for v1 users (or
implement a helper like isYarnBerry() that runs `yarn --version` and checks for
v2+), then return `yarn dlx ${script}` only when the helper reports Yarn Berry;
keep the same behavior for "npm", "bun", and "pnpm" branches and ensure the
detection helper is used by getRunCommand.
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 1-2: The type-only imports `Pool` and `StripeSdk` are being
referenced with `typeof Pool` and `typeof StripeSdk`, which is invalid for
type-only imports; update those references to use import-style typeof
expressions: replace `typeof Pool` with `typeof import("pg").Pool` and replace
`typeof StripeSdk` with `typeof import("stripe").default` wherever they appear
(e.g., the variables/types that reference `Pool` and `StripeSdk` in this module)
so the runtime types are correctly resolved.
In `@packages/paykit/src/cli/utils/update-check.ts`:
- Around line 7-22: The isNewer function currently only treats stable vs
prerelease and doesn't order two prerelease versions; update isNewer to, after
main version parity, handle prerelease comparison by splitting latestPre and
currentPre on "." into identifiers and comparing each identifier: treat numeric
identifiers as numbers and compare numerically, otherwise compare as ASCII
strings, and if all compared identifiers are equal the longer identifier list is
newer; keep the existing rule that a stable release (no prerelease) is newer
than any prerelease. This change should be implemented in the isNewer function
using the latest/current and latestPre/currentPre variables so cases like
"0.0.1-alpha.2" > "0.0.1-alpha.1" are correctly detected.
In `@packages/paykit/src/core/create-paykit.ts`:
- Around line 31-47: The current Promise.allSettled() call swallows rejections
from dryRunSyncProducts and relies on getPendingMigrationCount treating DB
errors as "all pending"; update the Promise.allSettled(...) block to inspect
each result, surface and log (or rethrow) any rejected promises instead of
ignoring them, and treat an error from getPendingMigrationCount(pool) as a real
error (log a clear DB/connectivity error or throw) rather than interpreting it
as a positive migration count; specifically modify the Promise.allSettled usage
around getPendingMigrationCount and dryRunSyncProducts to check
settled[i].status === "rejected" and handle its reason, and adjust
getPendingMigrationCount to propagate/throw errors instead of returning a
misleading count on failure.
- Around line 51-55: When creating the DB Pool from a connection string (the
const pool assigned when typeof options.database === "string") ensure the code
cleans up the owned Pool if createContext(...) rejects: wrap the await
createContext({ ...options, database: pool }) call in a try/catch, detect that
you own the Pool (i.e., it was created from options.database string), call
pool.end() (await it) in the catch to close sockets, then rethrow the original
error so startup still fails cleanly; reference the variables pool,
options.database and the createContext function when making this change.
In `@packages/paykit/src/utilities/dependencies/check-dependencies.ts`:
- Around line 11-17: The current logic in check-dependencies.ts uses
foundVersions to map a version to a single package name, causing subsequent
packages with the same version to be dropped and only one mismatch reported;
update the logic that iterates over resolved (the for (const [pkg, { version }]
of resolved) loop) so that foundVersions maps each version to an array of
package names (or otherwise accumulates duplicates) and then build the warning
and suggested install command using all package names for each mismatched
version (the same change should be applied to the later block around the 24-34
region), ensuring every package that shares a wrong version is included in the
reported message and fix command.
In `@packages/paykit/src/version.ts`:
- Line 1: Replace the unsupported named re-export of the JSON module by
importing the package.json default and then re-exporting the version property:
change the current `export { version } from "../package.json"` pattern to a
default import of `packageJson` (or similarly named identifier) from
`"../package.json"` and then export `version` as a named export (e.g., `export
const version = packageJson.version`) so the `version` symbol is provided at
runtime.
---
Outside diff comments:
In `@landing/content/docs/concepts/cli.mdx`:
- Around line 43-57: Remove the entire `paykitjs telemetry` subsection
(including the `<PackageRun command="paykitjs telemetry status" />` example),
the bullet list of telemetry subcommands (`enable`, `disable`, `status`), and
the paragraph about `PAYKIT_TELEMETRY_DISABLED` / `DO_NOT_TRACK` from the CLI
docs so no references to the removed `paykitjs telemetry` command remain; if you
want to keep users informed, replace that block with a single-line note stating
telemetry subcommands were removed and point to the relevant changelog or
migration doc.
---
Nitpick comments:
In `@e2e/cli/setup.ts`:
- Around line 9-10: The file currently sets process.env.PAYKIT_CLI = "1" at
module load which can leak across tests; change this to set and restore the
environment variable inside your test lifecycle (e.g., in your test fixture or
beforeEach/afterEach hooks): capture the original value of
process.env.PAYKIT_CLI, assign "1" at setup, and restore the original value in
cleanup so the mutation is scoped to the test. Update any related fixture file
or setup/teardown logic that uses process.env.PAYKIT_CLI to perform this
save/restore rather than assigning at module top-level.
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 91-94: The current diffs.map callback does a linear search with
plans.find for each diff causing O(n*m) complexity; instead, precompute a lookup
(e.g., const plansById = new Map(plans.map(p => [p.id, p])) ) before the map and
then inside the mapping use plansById.get(diff.id) to get the plan; update the
price calculation to use the found plan (plan?.priceAmount ?? 0 and
plan?.priceInterval) and keep the deps.formatPlanLine(diff.action, diff.id,
price) call unchanged.
In `@packages/paykit/src/utilities/dependencies/paykit-package-list.ts`:
- Line 1: The package array PAYKIT_PACKAGE_LIST should be made immutable to
prevent drift; change its export to use readonly literal tuples (e.g., export
const PAYKIT_PACKAGE_LIST = [...] as const) and update any install/check code to
consume that constant (or derive a string[] via Array.from or typeof
PAYKIT_PACKAGE_LIST[number] where needed) so all flows reuse the single
immutable source of truth.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 06149e67-900c-4a69-89c9-2410ecb772eb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
apps/demo/drizzle.config.tsapps/demo/package.jsonapps/demo/src/app/login/page.tsxapps/demo/src/app/page.tsxapps/demo/src/trpc/react.tsxdocs/superpowers/plans/2026-04-09-docs-content.mde2e/cli/setup.tse2e/cli/status.test.tslanding/content/docs/concepts/cli.mdxpackages/dash/package.jsonpackages/paykit/package.jsonpackages/paykit/src/cli/commands/check.tspackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/commands/status.tspackages/paykit/src/cli/commands/telemetry.tspackages/paykit/src/cli/index.tspackages/paykit/src/cli/utils/detect.tspackages/paykit/src/cli/utils/shared.tspackages/paykit/src/cli/utils/telemetry.tspackages/paykit/src/cli/utils/update-check.tspackages/paykit/src/core/create-paykit.tspackages/paykit/src/core/logger.tspackages/paykit/src/utilities/dependencies/check-dependencies.tspackages/paykit/src/utilities/dependencies/get-dependencies.tspackages/paykit/src/utilities/dependencies/index.tspackages/paykit/src/utilities/dependencies/paykit-package-list.tspackages/paykit/src/version.tspackages/stripe/package.json
💤 Files with no reviewable changes (2)
- packages/paykit/src/cli/commands/telemetry.ts
- packages/paykit/src/cli/commands/check.ts
Comment on lines
+7
to
+9
| dbCredentials: { | ||
| url: process.env.DATABASE_URL!, | ||
| }, |
There was a problem hiding this comment.
Avoid non-null assertion for DATABASE_URL in config.
Line 8 uses process.env.DATABASE_URL!; when unset, failure is less clear than an explicit config error.
🛠️ Suggested improvement
import { defineConfig } from "drizzle-kit";
import "dotenv/config";
+const databaseUrl = process.env.DATABASE_URL;
+if (!databaseUrl) {
+ throw new Error("Missing DATABASE_URL for Drizzle config");
+}
+
export default defineConfig({
dialect: "postgresql",
schema: "../../packages/paykit/src/database/schema.ts",
dbCredentials: {
- url: process.env.DATABASE_URL!,
+ url: databaseUrl,
},
migrations: {
schema: "public",
table: "paykit_migrations",
},
});📝 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.
Suggested change
| dbCredentials: { | |
| url: process.env.DATABASE_URL!, | |
| }, | |
| import { defineConfig } from "drizzle-kit"; | |
| import "dotenv/config"; | |
| const databaseUrl = process.env.DATABASE_URL; | |
| if (!databaseUrl) { | |
| throw new Error("Missing DATABASE_URL for Drizzle config"); | |
| } | |
| export default defineConfig({ | |
| dialect: "postgresql", | |
| schema: "../../packages/paykit/src/database/schema.ts", | |
| dbCredentials: { | |
| url: databaseUrl, | |
| }, | |
| migrations: { | |
| schema: "public", | |
| table: "paykit_migrations", | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/demo/drizzle.config.ts` around lines 7 - 9, Replace the non-null
assertion on process.env.DATABASE_URL in dbCredentials with an explicit runtime
check that throws a clear, descriptive error if DATABASE_URL is missing; update
the drizzle config's dbCredentials block to read the env var into a local
constant (e.g., databaseUrl) and validate it, then use that variable instead of
process.env.DATABASE_URL!, so any missing configuration surfaces as a helpful
error at startup.
Comment on lines
+34
to
35
| "./database": "./src/database/schema.ts" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package exports for ./database =="
jq -r '.exports["./database"], .publishConfig.exports["./database"].default, .publishConfig.exports["./database"].types' packages/paykit/package.json
echo
echo "== tsdown entry points =="
sed -n '1,120p' packages/paykit/tsdown.config.ts
echo
echo "== quick check for database schema entry presence =="
rg -n '"database|schema\.ts"' packages/paykit/tsdown.config.tsRepository: getpaykit/paykit
Length of output: 814
./database export points to unbuilt dist files.
The ./database export at lines 34 and 50 targets dist/database/schema.js and dist/database/schema.d.ts, but packages/paykit/tsdown.config.ts has no entry for src/database/schema.ts. This means the dist files will never be emitted during the build, breaking the paykitjs/database import in published packages.
Add the missing entry to tsdown.config.ts:
export default defineConfig({
...
entry: {
index: "src/index.ts",
"cli/index": "src/cli/index.ts",
"handlers/next": "src/handlers/next.ts",
"client/index": "src/client/index.ts",
+ "database/schema": "src/database/schema.ts",
},
...
});📝 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.
Suggested change
| "./database": "./src/database/schema.ts" | |
| }, | |
| export default defineConfig({ | |
| entry: { | |
| index: "src/index.ts", | |
| "cli/index": "src/cli/index.ts", | |
| "handlers/next": "src/handlers/next.ts", | |
| "client/index": "src/client/index.ts", | |
| "database/schema": "src/database/schema.ts", | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/package.json` around lines 34 - 35, The package exports
include an entry named "./database" that points to built artifacts that are
never emitted because tsdown.config.ts lacks an entry for
src/database/schema.ts; add a build entry in tsdown.config.ts for
src/database/schema.ts so the dist/database/schema.js and
dist/database/schema.d.ts are generated during the build and satisfy the
"./database" export (ensure the entry uses the same source path and will emit to
dist/database/schema.js/.d.ts).
Comment on lines
+33
to
49
| const [stripeResult, pendingMigrations] = await Promise.all([ | ||
| checkStripe(deps, config.options.provider.secretKey), | ||
| deps.getPendingMigrationCount(database), | ||
| ]); | ||
|
|
||
| if (!stripeResult.account.ok) { | ||
| s.stop(""); | ||
| p.log.error(`Stripe\n ${picocolors.red("✖")} ${stripeResult.account.message}`); | ||
| p.cancel("Push failed"); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Apply pending migrations before checking products | ||
| if (pendingMigrations > 0) { | ||
| await migrateDatabase(database); | ||
| p.log.success(`Schema ${picocolors.dim("·")} migrated`); | ||
| } else { | ||
| p.log.step(`Schema ${picocolors.dim("·")} up to date`); | ||
| s.message("Applying migrations"); | ||
| await deps.migrateDatabase(database); | ||
| } |
There was a problem hiding this comment.
Probe DB connectivity before treating migrations as pending.
getPendingMigrationCount() collapses query failures into “all migrations are pending”, so a broken database connection falls into the migration path here and only fails later in migrateDatabase(). That turns a clear connectivity error into a misleading migration flow. Reuse the explicit DB check from status before deciding whether to migrate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/src/cli/commands/push.ts` around lines 33 - 49, Before
treating a failure as "pending migrations", perform the explicit DB connectivity
check used in the status flow and bail early on connection errors: call the same
DB-check helper from the status command (the explicit connectivity check) before
invoking deps.getPendingMigrationCount(database); if the check reports a
connectivity/error condition, stop the spinner, log the DB error and exit
instead of proceeding to deps.migrateDatabase(database). Ensure you keep the
existing references to deps.getPendingMigrationCount and deps.migrateDatabase
but gate them behind the status-style DB connectivity check.
Comment on lines
+31
to
+47
| await Promise.allSettled([ | ||
| getPendingMigrationCount(pool).then((count) => { | ||
| if (count > 0) { | ||
| console.warn( | ||
| `${picocolors.yellow("[paykit]")} ${count} pending migration${count === 1 ? "" : "s"}. Run ${picocolors.bold("paykitjs push")} to apply.`, | ||
| ); | ||
| } | ||
| }), | ||
| dryRunSyncProducts(ctx).then((results) => { | ||
| const outOfSync = results.filter((r) => r.action !== "unchanged"); | ||
| if (outOfSync.length > 0) { | ||
| ctx.logger.error( | ||
| `${outOfSync.length} plan(s) out of sync: ${outOfSync.map((r) => r.id).join(", ")}. Run \`paykitjs push\` to update.`, | ||
| console.warn( | ||
| `${picocolors.yellow("[paykit]")} ${outOfSync.length} product${outOfSync.length === 1 ? "" : "s"} out of sync: ${outOfSync.map((r) => r.id).join(", ")}. Run ${picocolors.bold("paykitjs push")} to update.`, | ||
| ); | ||
| } | ||
| } catch { | ||
| ctx.logger.debug("Skipped plan sync check (database may not be initialized yet)"); | ||
| } | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
Don't silently hide failed dev checks.
This path never inspects the Promise.allSettled() results, so a rejected dryRunSyncProducts(ctx) disappears completely. On top of that, getPendingMigrationCount(pool) treats query failures as “all migrations pending”, so a bad DB connection is reported as a migration issue instead of a connectivity failure. That means the new dev checks can mask the real problem instead of surfacing it clearly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/src/core/create-paykit.ts` around lines 31 - 47, The current
Promise.allSettled() call swallows rejections from dryRunSyncProducts and relies
on getPendingMigrationCount treating DB errors as "all pending"; update the
Promise.allSettled(...) block to inspect each result, surface and log (or
rethrow) any rejected promises instead of ignoring them, and treat an error from
getPendingMigrationCount(pool) as a real error (log a clear DB/connectivity
error or throw) rather than interpreting it as a positive migration count;
specifically modify the Promise.allSettled usage around getPendingMigrationCount
and dryRunSyncProducts to check settled[i].status === "rejected" and handle its
reason, and adjust getPendingMigrationCount to propagate/throw errors instead of
returning a misleading count on failure.
Comment on lines
+51
to
+55
| const pool = | ||
| typeof options.database === "string" | ||
| ? new Pool({ connectionString: options.database }) | ||
| : options.database; | ||
| const ctx = await createContext({ ...options, database: pool }); |
There was a problem hiding this comment.
Dispose the owned Pool when context creation fails.
When options.database is a connection string, this allocates a new pool before createContext(). If createContext() rejects, nothing closes that pool, so startup failures leak sockets until process exit.
♻️ Suggested fix
async function initContext(options: PayKitOptions): Promise<PayKitContext> {
- const pool =
- typeof options.database === "string"
- ? new Pool({ connectionString: options.database })
- : options.database;
- const ctx = await createContext({ ...options, database: pool });
+ const ownsPool = typeof options.database === "string";
+ const pool = ownsPool
+ ? new Pool({ connectionString: options.database })
+ : options.database;
+
+ let ctx: PayKitContext;
+ try {
+ ctx = await createContext({ ...options, database: pool });
+ } catch (error) {
+ if (ownsPool) {
+ await pool.end();
+ }
+ throw error;
+ }
if (process.env.NODE_ENV !== "production" && !process.env.PAYKIT_CLI) {
runDevChecks(ctx, pool).catch(() => {});
}📝 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.
Suggested change
| const pool = | |
| typeof options.database === "string" | |
| ? new Pool({ connectionString: options.database }) | |
| : options.database; | |
| const ctx = await createContext({ ...options, database: pool }); | |
| const ownsPool = typeof options.database === "string"; | |
| const pool = ownsPool | |
| ? new Pool({ connectionString: options.database }) | |
| : options.database; | |
| let ctx: PayKitContext; | |
| try { | |
| ctx = await createContext({ ...options, database: pool }); | |
| } catch (error) { | |
| if (ownsPool) { | |
| await pool.end(); | |
| } | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/src/core/create-paykit.ts` around lines 51 - 55, When
creating the DB Pool from a connection string (the const pool assigned when
typeof options.database === "string") ensure the code cleans up the owned Pool
if createContext(...) rejects: wrap the await createContext({ ...options,
database: pool }) call in a try/catch, detect that you own the Pool (i.e., it
was created from options.database string), call pool.end() (await it) in the
catch to close sockets, then rethrow the original error so startup still fails
cleanly; reference the variables pool, options.database and the createContext
function when making this change.
packages/paykit/src/utilities/dependencies/check-dependencies.ts
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1 @@ | |||
| export { version } from "../package.json"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TypeScript 5.9 + Node ESM, is export { version } from "../package.json" reliably supported, or should JSON be default-imported and then re-exported as a const?
💡 Result:
No, export { version } from "../package.json" is not reliably supported in TypeScript 5.9 with Node.js ESM. JSON modules in Node.js only provide a default export, not named exports like version. Attempting named imports or re-exports from JSON files results in runtime errors such as "The requested module does not provide an export named 'version'". TypeScript's nodenext/node20 modes enforce this by erroring on named imports from JSON (TS1544) and requiring with { type: "json" } for imports. The reliable approach is to default-import the JSON and re-export as a named const: import pkg from "../package.json" with { type: "json" }; export const version = pkg.version; This works at runtime in Node.js (v18.20+ stable JSON modules) and compiles correctly in TS 5.9 under module: "nodenext" or "node20", providing a named export version with proper typing.
Citations:
- 1: https://nodejs.org/api/esm.html
- 2: Validate JSON imports into ESM in
--module nodenextmicrosoft/TypeScript#60019 - 3: import {version} from '../package.json' evanw/esbuild#1486
- 4: ESM modules and json importing [feature request] nodejs/node#20494
🏁 Script executed:
# Check for tsconfig.json and its module settings
find . -name "tsconfig.json" -type f | head -5Repository: getpaykit/paykit
Length of output: 191
🏁 Script executed:
# Search for existing JSON imports in the codebase
rg "import.*from.*\.json" -t ts -t tsx --max-count 20Repository: getpaykit/paykit
Length of output: 87
🏁 Script executed:
# Check package.json for any JSON module type or import assertions
cat package.json | grep -A 5 -B 5 "json\|module\|type"Repository: getpaykit/paykit
Length of output: 1581
🏁 Script executed:
# Check the paykit tsconfig.json for module configuration
cat ./packages/paykit/tsconfig.jsonRepository: getpaykit/paykit
Length of output: 174
🏁 Script executed:
# Search for existing JSON imports in the codebase (corrected syntax)
rg "import.*from.*\.json" -t ts --max-count 20Repository: getpaykit/paykit
Length of output: 153
🏁 Script executed:
# Also search for any JSON import assertions
rg "import.*\.json.*with" -t ts --max-count 20Repository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
# Check the base tsconfig.json
cat ./tsconfig.base.jsonRepository: getpaykit/paykit
Length of output: 867
🏁 Script executed:
# Check the landing/src/lib/community-stats.ts file to see the JSON import pattern
cat -n ./landing/src/lib/community-stats.ts | head -20Repository: getpaykit/paykit
Length of output: 643
Named re-export from JSON is not supported and will fail at runtime.
Line 1 attempts export { version } from "../package.json", but JSON modules in Node.js only provide a default export. This will cause a runtime error: "The requested module does not provide an export named 'version'". The codebase already uses the correct pattern (see landing/src/lib/community-stats.ts).
Use a default import and explicit named export instead:
Fix
-export { version } from "../package.json";
+import pkg from "../package.json";
+
+export const version = pkg.version;📝 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.
Suggested change
| export { version } from "../package.json"; | |
| import pkg from "../package.json"; | |
| export const version = pkg.version; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/src/version.ts` at line 1, Replace the unsupported named
re-export of the JSON module by importing the package.json default and then
re-exporting the version property: change the current `export { version } from
"../package.json"` pattern to a default import of `packageJson` (or similarly
named identifier) from `"../package.json"` and then export `version` as a named
export (e.g., `export const version = packageJson.version`) so the `version`
symbol is provided at runtime.
- Fix check-dependencies to track all packages per version, not just first - Improve isNewer to compare pre-release identifiers properly - Add provider existence check in push command before calling Stripe - Precompute plan lookup Map in formatProductDiffs - Make PAYKIT_PACKAGE_LIST immutable with as const - Update CLI docs to remove stale telemetry command section
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/paykit/src/cli/utils/shared.ts (1)
19-21:⚠️ Potential issue | 🔴 CriticalFix invalid
typeofon type-only imports.Line 20 and Line 21 use
typeofon symbols imported withimport type, which is invalid in TypeScript and will fail type-checking.Proposed fix
export interface CliDeps { - Pool: typeof Pool; - StripeSdk: typeof StripeSdk; + Pool: typeof import("pg").Pool; + StripeSdk: typeof import("stripe").default; createContext: typeof createContext;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/cli/utils/shared.ts` around lines 19 - 21, The CliDeps interface incorrectly uses "typeof" on type-only imports (Pool and StripeSdk); remove the invalid "typeof" and reference the types directly in the CliDeps interface (i.e., change "Pool: typeof Pool" and "StripeSdk: typeof StripeSdk" to "Pool: Pool" and "StripeSdk: StripeSdk"), or alternatively convert those imports from "import type" to regular value imports if you actually need the runtime constructors — update the declarations in the CliDeps interface accordingly (referencing the CliDeps interface, Pool and StripeSdk symbols).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 19-21: The CliDeps interface incorrectly uses "typeof" on
type-only imports (Pool and StripeSdk); remove the invalid "typeof" and
reference the types directly in the CliDeps interface (i.e., change "Pool:
typeof Pool" and "StripeSdk: typeof StripeSdk" to "Pool: Pool" and "StripeSdk:
StripeSdk"), or alternatively convert those imports from "import type" to
regular value imports if you actually need the runtime constructors — update the
declarations in the CliDeps interface accordingly (referencing the CliDeps
interface, Pool and StripeSdk symbols).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 983ef385-dcc7-4826-a577-560e98ae4c2d
📒 Files selected for processing (7)
docs/superpowers/plans/2026-04-09-docs-content.mdlanding/content/docs/concepts/cli.mdxpackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/utils/shared.tspackages/paykit/src/cli/utils/update-check.tspackages/paykit/src/utilities/dependencies/check-dependencies.tspackages/paykit/src/utilities/dependencies/paykit-package-list.ts
✅ Files skipped from review due to trivial changes (1)
- packages/paykit/src/utilities/dependencies/paykit-package-list.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/superpowers/plans/2026-04-09-docs-content.md
- landing/content/docs/concepts/cli.mdx
- packages/paykit/src/utilities/dependencies/check-dependencies.ts
- packages/paykit/src/cli/utils/update-check.ts
- packages/paykit/src/cli/commands/push.ts
This was referenced Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
●sections, single spinner)PAYKIT_TELEMETRY_DISABLED=1/DO_NOT_TRACK=1), remove~/.paykit/config dir and telemetry subcommand@paykitjs/*packages at dev startupTest plan
pnpm paykitjs statusshows consistent output with●sectionspnpm paykitjs pushshows consistent output, confirmation prompt, and sync result in outro@paykitjs/*packages are on different versionspnpm typecheckpassespnpm lintpasses (no new warnings)Summary by CodeRabbit
New Features
Changes
--throwChores