Feat/multi item subscriptions - #204
Conversation
|
@sirfusebox is attempting to deploy a commit to the maxktz Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe change adds combined checkout and subscription add-on support, Stripe subscription-item tracking, metered pricing synchronization, usage reporting, webhook reconciliation, documentation, publishing automation, and end-to-end coverage. ChangesSubscription billing extensions
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/paykit/src/stripe/stripe-provider.ts (1)
839-849: 🎯 Functional Correctness | 🟠 MajorHandle metered target prices in
updateSubscription.
updateSubscriptioncurrently passes onlyprice, but update item documentation resets the item quantity to1when price changes. A change from an existing licensed price to a metered price therefore creates an invalid metered item with quantity and can fail. MatchcreateSubscriptionandscheduleSubscriptionChangeby retrieving the target price first, then sending noquantityfor metered prices while preserving quantity for licensed target prices. Validatedata.providerProduct.priceIdhere too.[low_offort_and_high_reward]
🤖 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 `@packages/paykit/src/stripe/stripe-provider.ts` around lines 839 - 849, The updateSubscription flow must retrieve and validate data.providerProduct.priceId before updating the subscription item, then determine whether the target price is metered. In the client.subscriptions.update call, omit quantity for metered prices and preserve the existing quantity for licensed prices, matching createSubscription and scheduleSubscriptionChange behavior.
🧹 Nitpick comments (7)
e2e/core/subscribe/downgrade-with-addon.test.ts (1)
88-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFilter the add-on row by status for a deterministic result.
The query uses
limit(1)without a status filter or an order. If more than oneextra_messagesrow exists, the selected row is nondeterministic and can carry a stalestripeSubscriptionId. The upgrade test appliesne(subscription.status, "ended")for the same lookup. Apply the same filter here.♻️ Proposed change
-import { and, eq } from "drizzle-orm"; +import { and, eq, ne } from "drizzle-orm";- .where(and(eq(subscription.customerId, customerId), eq(product.id, "extra_messages"))) + .where( + and( + eq(subscription.customerId, customerId), + eq(product.id, "extra_messages"), + ne(subscription.status, "ended"), + ), + ) .limit(1);🤖 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 `@e2e/core/subscribe/downgrade-with-addon.test.ts` around lines 88 - 97, Update the add-on lookup query in the downgrade test to include the same non-ended status predicate used by the upgrade lookup, alongside the existing customer and product filters. Keep the limit and stripeSubscriptionId validation unchanged.packages/paykit/src/database/migrations/0004_add_metered_pricing.sql (1)
3-3: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd the foreign key as
NOT VALID, then validate it separately.
ADD CONSTRAINT ... FOREIGN KEYtakes aSHARE ROW EXCLUSIVElock onpaykit_productandpaykit_featureand scans the table to validate existing rows. Every existing row hasmetered_feature_idNULL, so the scan finds nothing, but writes to both tables are still blocked while it runs. Split the operation so the lock is short.🛠️ Proposed split
-ALTER TABLE "paykit_product" ADD CONSTRAINT "paykit_product_metered_feature_id_paykit_feature_id_fk" FOREIGN KEY ("metered_feature_id") REFERENCES "public"."paykit_feature"("id") ON DELETE no action ON UPDATE no action; +ALTER TABLE "paykit_product" ADD CONSTRAINT "paykit_product_metered_feature_id_paykit_feature_id_fk" FOREIGN KEY ("metered_feature_id") REFERENCES "public"."paykit_feature"("id") ON DELETE no action ON UPDATE no action NOT VALID;--> statement-breakpoint +ALTER TABLE "paykit_product" VALIDATE CONSTRAINT "paykit_product_metered_feature_id_paykit_feature_id_fk";If Drizzle generated this file, apply the change in the generator input or accept the short lock and document it.
🤖 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 `@packages/paykit/src/database/migrations/0004_add_metered_pricing.sql` at line 3, Update the paykit_product_metered_feature_id_paykit_feature_id_fk migration to add the foreign key with NOT VALID, then run a separate VALIDATE CONSTRAINT statement for that constraint. If this SQL is generated by Drizzle, apply the equivalent change in its migration generator input instead.Source: Linters/SAST tools
packages/paykit/src/stripe/stripe-provider.ts (1)
260-268: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache
isMeteredPriceIdresults.
isMeteredPriceIdperforms aprices.retrievecall on every invocation.scheduleSubscriptionChangeat Line 969 andcreateSubscriptionCheckoutat Line 751 call it on request paths, once per price. A price'srecurring.usage_typeis immutable in Stripe, so a per-providerMap<string, boolean>cache removes repeated round trips. Note thatisMeteredSubscriptionItemPricealready avoids the call when the price object is expanded; prefer it wherever the expanded item is available.🤖 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 `@packages/paykit/src/stripe/stripe-provider.ts` around lines 260 - 268, The isMeteredPriceId helper performs repeated Stripe price lookups for immutable usage metadata. Add a per-provider Map<string, boolean> cache, reuse cached results before calling client.prices.retrieve, and store each retrieved usage result; in scheduleSubscriptionChange and createSubscriptionCheckout, use isMeteredSubscriptionItemPrice whenever an expanded price object is available.packages/paykit/src/product/product.service.ts (1)
205-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPropagate
PriceUsageTypeinstead of widening it tostring.insertProductVersiontypespriceUsageTypeasstring, so the union defined inpackages/paykit/src/types/schema.tsLine 66 is lost at the persistence boundary. The consumer then has to reassert it with a cast, which silently accepts any string value that reaches the database.
packages/paykit/src/product/product.service.ts#L205-L210: typepriceUsageTypeasPriceUsageTypein theinsertProductVersioninput, and use the same type onStoredProduct.packages/paykit/src/product/product-sync.service.ts#L134-L135: declareusageType: PriceUsageTypeon thepaidPlansToSyncelement type.packages/paykit/src/product/product-sync.service.ts#L240-L240: remove theas "licensed" | "metered"cast once the type flows through.As per coding guidelines "Use
import typefor type-only imports" — importPriceUsageTypewithimport type.🤖 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 `@packages/paykit/src/product/product.service.ts` around lines 205 - 210, Propagate the schema PriceUsageType union through product persistence: in packages/paykit/src/product/product.service.ts lines 205-210, import it with import type and use it for both insertProductVersion’s priceUsageType input and StoredProduct; in packages/paykit/src/product/product-sync.service.ts lines 134-135, type paidPlansToSync usageType as PriceUsageType, and at line 240 remove the redundant "licensed" | "metered" cast.Source: Coding guidelines
packages/paykit/src/product/__tests__/product.service.test.ts (1)
18-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where the metering fields differ.
Both fixtures default to
meteredFeatureId: nullandpriceUsageType: "licensed". The new comparisons inproductSnapshotMatchesPlanatpackages/paykit/src/product/product.service.tsLines 146-147 are therefore never exercised with differing values. A single case where the stored product is"licensed"and the plan is"metered"would assert thatgetProductByPlanreturnsnull.💚 Suggested test
it("does not match a stored licensed product against a metered plan", async () => { // arrange getLatestProductSnapshot to return createStoredProduct() const plan = createPlan({ priceUsageType: "metered", meteredFeatureId: "api_calls" }); await expect(getProductByPlan(database, plan)).resolves.toBeNull(); });Also applies to: 39-44
🤖 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 `@packages/paykit/src/product/__tests__/product.service.test.ts` around lines 18 - 23, Add a test covering differing metering fields in the product lookup tests: keep the stored fixture licensed with no metered feature, create a plan with priceUsageType "metered" and a non-null meteredFeatureId, and assert getProductByPlan resolves to null. Ensure the test exercises the comparisons in productSnapshotMatchesPlan.packages/paykit/src/database/migrations/0003_add_subscription_item_id.sql (1)
2-3: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winIndex creation blocks writes, and the unique index does not constrain legacy rows.
Two points on this migration:
- Both
CREATE INDEXstatements take anACCESS EXCLUSIVE-style lock that blocks writes topaykit_subscriptionfor the duration of the build. On a large table this is a write outage. UseCREATE INDEX CONCURRENTLYin a migration that runs outside a transaction.stripe_subscription_item_idis NULL for every pre-existing row, and Postgres treats NULLs as distinct in a unique index. Sopaykit_subscription_stripe_sub_item_uniquedoes not prevent two legacy rows from sharing the samestripe_subscription_id. If the intent is one row per subscription while the item id is unknown, addNULLS NOT DISTINCT(Postgres 15 and later).Confirm the minimum supported Postgres version before choosing
NULLS NOT DISTINCT.#!/bin/bash # Check declared Postgres version support and whether Drizzle runs migrations in a transaction. rg -nP -C3 '(postgres|pg)[^\n]*(1[2-9]|version)' --iglob '*.{json,yml,yaml,toml,ts,md}' -g '!**/node_modules/**' | head -50 fd -t f 'drizzle.config' --exec cat🤖 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 `@packages/paykit/src/database/migrations/0003_add_subscription_item_id.sql` around lines 2 - 3, Update the indexes paykit_subscription_stripe_item_idx and paykit_subscription_stripe_sub_item_unique to build concurrently, and configure this migration to run outside a transaction. Confirm the minimum supported PostgreSQL version first; if it is PostgreSQL 15 or newer, make the unique index treat NULL stripe_subscription_item_id values as not distinct so legacy rows remain constrained. If older versions are supported, use a compatible approach that enforces the intended uniqueness without NULLS NOT DISTINCT.Source: Linters/SAST tools
packages/paykit/src/types/schema.ts (1)
35-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake ambiguous price inputs fail validation.
An object with both
amount/intervalandunitAmount/meteredBymatcheslicensedPriceSchemafirst in the union because Zod object schemas strip unknown keys by default. Use strict object schemas for both price branches so Zod rejects ambiguous configurations; if output keys matter after parsing, restore the original fields or use passthrough at the union level.🤖 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 `@packages/paykit/src/types/schema.ts` at line 35, Update licensedPriceSchema and meteredPriceSchema to use strict object validation so inputs containing fields from both price variants are rejected instead of being accepted after unknown-key stripping. Preserve each schema’s intended output fields, and keep priceSchema’s union behavior unchanged for valid licensed or metered inputs.
🤖 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 `@e2e/core/checkout/renewal-after-combined-checkout.test.ts`:
- Around line 62-63: Guard rowsBefore[0]!.currentPeriodEndAt before constructing
periodEnd, asserting or failing clearly when it is null, and pass the nullable
timestamp directly to Date without the unnecessary double cast. Keep advanceTo
based on the validated periodEnd so the test never advances to the Unix epoch.
In `@e2e/core/webhook/subscription-deleted-multi-item.test.ts`:
- Around line 47-56: Update the subscription lookup using the visible subRows
query to filter for non-null subscription.stripeSubscriptionId before ordering
and limiting, so the selected row always has a Stripe subscription ID while
preserving the existing customerId and newest-row selection.
In `@e2e/test-utils/products.ts`:
- Around line 52-57: Update the price configuration in meteredUsagePlan so the
fractional rate is represented through the Stripe-compatible decimal
metered-price field, such as unit_amount_decimal, rather than unitAmount.
Preserve the existing monthly interval, apiCallsFeature metering, and 0.01 rate.
In `@packages/paykit/src/providers/provider.ts`:
- Line 111: Make addSubscriptionItem, removeSubscriptionItem, reportUsageEvent,
and reportsUsageEvent optional in PaymentProvider, then update their consumers
to handle missing implementations by returning a clear PayKit
unsupported-provider error. Ensure Stripe’s implementations remain unchanged and
partial custom providers or test doubles remain type-compatible.
In `@packages/paykit/src/stripe/stripe-provider.ts`:
- Around line 869-875: Update addSubscriptionItem to reject a missing
data.providerProduct.priceId using the same PayKit validation as
createSubscription, then call isMeteredPriceId and omit quantity for metered
add-on prices while preserving the existing quantity behavior for non-metered
prices.
- Around line 917-926: Update the removeSubscriptionItem flow after
retrieveExpandedSubscription so it returns subscription-level state without
normalizing to an arbitrary remaining item via normalizeStripeSubscription.
Ensure the result does not expose another item's providerSubscriptionItemId, or
clearly document and enforce that callers ignore that field.
- Around line 745-762: In the checkout session flow surrounding the
providerProducts mapping and lineItems construction, explicitly reject an empty
providerProducts array before calling Stripe. Throw the appropriate PayKit
BAD_REQUEST error using the existing provider-price-required error code, while
preserving the current validation for missing individual priceId values.
- Around line 1114-1121: Update the recurring-price construction around the
metered-product branch in stripe-provider.ts to treat usageType "metered"
without a meterEventName as a configuration error. Validate this before the
licensed-price else-if path, surface a clear error, and only call
ensureStripeMeter when the required meterEventName is present.
- Around line 213-223: Update normalizeStripeSubscriptionItem so items[0] is
used only when itemId is absent; when itemId is provided, return only the
matching item and preserve the existing behavior for an omitted itemId. Ensure
callers passing a known item ID cannot receive an unrelated subscription item.
- Around line 964-974: Update the subscription phase-item construction around
currentItems and nextPhaseItems to iterate through all records from
client.subscriptionItems.list rather than relying on expanded
currentSub.items.data, preserving every add-on while changing only targetItem.
Also update normalizeStripeSubscriptionItems and every consumer of expanded
items.data to paginate through all subscription-item pages before processing
them.
- Around line 270-286: Update ensureStripeMeter to traverse all active billing
meter pages using the Stripe Node SDK pagination helper, then search the
complete result set for a matching event_name before calling
billing.meters.create. Preserve returning the existing meter ID when found and
only create a meter when no active match exists.
In `@packages/paykit/src/subscription/subscription.service.ts`:
- Around line 1272-1274: Update the target subscription resolution around the
input.targetSubscriptionId branch to validate it against an active local
subscription row owned by input.customerId before returning any provider ID.
Return the matched row’s stripeSubscriptionId, and reject or fail resolution
when no customer-scoped active row matches instead of passing the
client-supplied ID to the provider.
- Around line 1317-1342: Update the add-on attachment flow around the
activeSubscription check and provider.addSubscriptionItem so concurrent requests
reserve the customer/plan attachment before the provider call using the existing
uniqueness constraint or locking mechanism. Reuse that reservation to prevent
duplicate local records, and pass a stable idempotency key to
provider.addSubscriptionItem so retries or races cannot create multiple provider
items.
- Around line 248-250: Update the duplicate-group validation in the combined
subscription flow to exclude empty group values before constructing the Set and
comparing lengths. Preserve duplicate detection for non-empty groups while
allowing multiple ungrouped plans, consistent with loadSubscribeContext’s
treatment of an empty group as no group.
In `@packages/paykit/src/subscription/subscription.types.ts`:
- Around line 8-9: Update the addOnPlanIds schema in the subscription types to
reject duplicate plan IDs, including input.planId, before
handleCombinedSubscribe proceeds, and apply a Zod max-length limit to cap
add-ons before checkout. Ensure invalid input cannot trigger paymentUrl creation
or concurrent loadSubscribeContext and metadata work.
In `@packages/paykit/src/types/schema.ts`:
- Around line 19-35: Update meteredPriceSchema so unitAmount cannot accept
positive values that normalize to zero cents, while keeping priceAmountSchema
unchanged for licensed prices. Add a schema-level validation for metered
unitAmount values requiring at least one cent, preventing syncProducts from
producing a Stripe unit_amount of zero.
- Around line 445-457: Update the plan normalization logic around metered price
handling to verify that price.meteredBy.id references a feature included by the
same plan before assigning meteredFeatureId. Reject invalid references with a
clear configuration error during normalizeSchema, while preserving the existing
metered pricing values for valid references.
In `@packages/paykit/src/usage/usage.service.ts`:
- Around line 19-22: Update the eligibility query in the usage service to
require the product’s priceUsageType to equal "metered" alongside the existing
meteredFeatureId condition. Use the schema’s priceUsageType field and preserve
all other subscription and expiration filters.
---
Outside diff comments:
In `@packages/paykit/src/stripe/stripe-provider.ts`:
- Around line 839-849: The updateSubscription flow must retrieve and validate
data.providerProduct.priceId before updating the subscription item, then
determine whether the target price is metered. In the
client.subscriptions.update call, omit quantity for metered prices and preserve
the existing quantity for licensed prices, matching createSubscription and
scheduleSubscriptionChange behavior.
---
Nitpick comments:
In `@e2e/core/subscribe/downgrade-with-addon.test.ts`:
- Around line 88-97: Update the add-on lookup query in the downgrade test to
include the same non-ended status predicate used by the upgrade lookup,
alongside the existing customer and product filters. Keep the limit and
stripeSubscriptionId validation unchanged.
In `@packages/paykit/src/database/migrations/0003_add_subscription_item_id.sql`:
- Around line 2-3: Update the indexes paykit_subscription_stripe_item_idx and
paykit_subscription_stripe_sub_item_unique to build concurrently, and configure
this migration to run outside a transaction. Confirm the minimum supported
PostgreSQL version first; if it is PostgreSQL 15 or newer, make the unique index
treat NULL stripe_subscription_item_id values as not distinct so legacy rows
remain constrained. If older versions are supported, use a compatible approach
that enforces the intended uniqueness without NULLS NOT DISTINCT.
In `@packages/paykit/src/database/migrations/0004_add_metered_pricing.sql`:
- Line 3: Update the paykit_product_metered_feature_id_paykit_feature_id_fk
migration to add the foreign key with NOT VALID, then run a separate VALIDATE
CONSTRAINT statement for that constraint. If this SQL is generated by Drizzle,
apply the equivalent change in its migration generator input instead.
In `@packages/paykit/src/product/__tests__/product.service.test.ts`:
- Around line 18-23: Add a test covering differing metering fields in the
product lookup tests: keep the stored fixture licensed with no metered feature,
create a plan with priceUsageType "metered" and a non-null meteredFeatureId, and
assert getProductByPlan resolves to null. Ensure the test exercises the
comparisons in productSnapshotMatchesPlan.
In `@packages/paykit/src/product/product.service.ts`:
- Around line 205-210: Propagate the schema PriceUsageType union through product
persistence: in packages/paykit/src/product/product.service.ts lines 205-210,
import it with import type and use it for both insertProductVersion’s
priceUsageType input and StoredProduct; in
packages/paykit/src/product/product-sync.service.ts lines 134-135, type
paidPlansToSync usageType as PriceUsageType, and at line 240 remove the
redundant "licensed" | "metered" cast.
In `@packages/paykit/src/stripe/stripe-provider.ts`:
- Around line 260-268: The isMeteredPriceId helper performs repeated Stripe
price lookups for immutable usage metadata. Add a per-provider Map<string,
boolean> cache, reuse cached results before calling client.prices.retrieve, and
store each retrieved usage result; in scheduleSubscriptionChange and
createSubscriptionCheckout, use isMeteredSubscriptionItemPrice whenever an
expanded price object is available.
In `@packages/paykit/src/types/schema.ts`:
- Line 35: Update licensedPriceSchema and meteredPriceSchema to use strict
object validation so inputs containing fields from both price variants are
rejected instead of being accepted after unknown-key stripping. Preserve each
schema’s intended output fields, and keep priceSchema’s union behavior unchanged
for valid licensed or metered inputs.
🪄 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 Plus
Run ID: b0c606fc-bbbe-4ec2-879a-8908a3e144d4
📒 Files selected for processing (44)
.gitattributesapps/web/content/docs/database.mdxapps/web/content/docs/entitlements.mdxapps/web/content/docs/metered-usage.mdxapps/web/content/docs/plans-and-features.mdxapps/web/content/docs/subscriptions.mdxapps/web/content/docs/webhook-events.mdxe2e/core/addon/add-addon.test.tse2e/core/addon/remove-addon-out-of-band.test.tse2e/core/addon/remove-addon.test.tse2e/core/addon/remove-last-item-rejected.test.tse2e/core/checkout/combined-checkout.test.tse2e/core/checkout/renewal-after-combined-checkout.test.tse2e/core/subscribe/downgrade-with-addon.test.tse2e/core/subscribe/upgrade-with-addon.test.tse2e/core/usage/metered-plan-sync.test.tse2e/core/usage/report-usage-not-metered.test.tse2e/core/usage/report-usage.test.tse2e/core/webhook/subscription-deleted-multi-item.test.tse2e/test-utils/products.tse2e/test-utils/setup.tspackage.jsonpackages/paykit/src/api/methods.tspackages/paykit/src/core/errors.tspackages/paykit/src/database/migrations/0003_add_subscription_item_id.sqlpackages/paykit/src/database/migrations/0004_add_metered_pricing.sqlpackages/paykit/src/database/migrations/meta/0003_snapshot.jsonpackages/paykit/src/database/migrations/meta/0004_snapshot.jsonpackages/paykit/src/database/migrations/meta/_journal.jsonpackages/paykit/src/database/schema.tspackages/paykit/src/product/__tests__/product.service.test.tspackages/paykit/src/product/product-sync.service.tspackages/paykit/src/product/product.service.tspackages/paykit/src/providers/provider.tspackages/paykit/src/stripe/stripe-provider.tspackages/paykit/src/subscription/addon.api.tspackages/paykit/src/subscription/subscription.api.tspackages/paykit/src/subscription/subscription.service.tspackages/paykit/src/subscription/subscription.types.tspackages/paykit/src/types/events.tspackages/paykit/src/types/schema.tspackages/paykit/src/usage/usage.api.tspackages/paykit/src/usage/usage.service.tspackages/paykit/src/webhook/webhook.service.ts
| export const meteredUsagePlan = plan({ | ||
| group: "usage", | ||
| id: "metered_usage", | ||
| name: "Metered Usage", | ||
| price: { interval: "month", meteredBy: apiCallsFeature, unitAmount: 0.01 }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the metered price contract and the Stripe conversion path.
rg -n 'unitAmount|unit_amount_decimal|meteredBy' -g '*.ts' -C4 packages/paykit/srcRepository: getpaykit/paykit
Length of output: 2776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Schema definitions"
sed -n '1,120p' packages/paykit/src/types/schema.ts
echo "## Metered price conversion region"
sed -n '420,500p' packages/paykit/src/types/schema.ts
echo "## Stripe provider files"
fd -a 'stripe|price|sync' packages/paykit/src | sed 's#^\./##' | head -80
echo "## Search Stripe price creation fields"
rg -n 'create\\(|price|unit_amount|unit_amount_decimal|currency|amount' -g '*.ts' packages/paykit/src -C3Repository: getpaykit/paykit
Length of output: 7380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Stripe provider"
wc -l packages/paykit/src/stripe/stripe-provider.ts
sed -n '1,260p' packages/paykit/src/stripe/stripe-provider.ts
echo "## Product sync"
wc -l packages/paykit/src/product/product-sync.service.ts
sed -n '1,260p' packages/paykit/src/product/product-sync.service.ts
echo "## Stripe tests relevant snippets"
wc -l packages/paykit/src/stripe/__tests__/stripe-provider.test.ts
rg -n 'unit_amount|unit_amount_decimal|unitAmount|0\.01|metered|price' -g '*.ts' packages/paykit/src/stripe packages/paykit/src/product -C3
echo "## Schema migration money fields"
rg -n 'priceAmount|metered|priceCurrency|priceInterval|priceUsageType' -g '*.sql' packages/paykit/src/database/migrations -C3Repository: getpaykit/paykit
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Stripe provider product sync implementation"
sed -n '1084,1130p' packages/paykit/src/stripe/stripe-provider.ts
echo "## Behavioral probe: schema normalization for fractional unitAmount"
python3 - <<'PY'
from decimal import Decimal
def normalize(unit_amount_dollars: float | int | None) -> int | None:
return None if unit_amount_dollars is None else int(Decimal(str(unit_amount_dollars)) * 100).to_integral_exact(rounding="ROUND_HALF_EVEN")
for value in [0.01, 0.001, 0.1, 0.29, 0.31]:
print(value, "=>", normalize(value))
print("Stripe integer cents field would receive:", normalize(0.01))
PYRepository: getpaykit/paykit
Length of output: 2224
Use unit_amount_decimal for metered prices with fractional rates.
unitAmount: 0.01 is accepted by the pricing schema, but normalization converts 0.01 to integer cents as 1 and Stripe sync passes unit_amount: 1 for metered prices. Stripe prices require unit_amount: 0 for sub-cent rates. Set Stripe metered prices through unit_amount_decimal or another sub-cent field.
🤖 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 `@e2e/test-utils/products.ts` around lines 52 - 57, Update the price
configuration in meteredUsagePlan so the fractional rate is represented through
the Stripe-compatible decimal metered-price field, such as unit_amount_decimal,
rather than unitAmount. Preserve the existing monthly interval, apiCallsFeature
metering, and 0.01 rate.
| const price = exportedPlan.price; | ||
| let priceUsageType: PriceUsageType = "licensed"; | ||
| let meteredFeatureId: string | null = null; | ||
| let priceAmountDollars: number | null = null; | ||
| if (price) { | ||
| if (isMeteredPrice(price)) { | ||
| priceUsageType = "metered"; | ||
| meteredFeatureId = price.meteredBy.id; | ||
| priceAmountDollars = price.unitAmount; | ||
| } else { | ||
| priceAmountDollars = price.amount; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
meteredBy is not verified against the plan's includes, so the product FK can fail at sync time.
meteredFeatureId is taken directly from price.meteredBy.id. Feature rows are upserted only from ctx.products.features, which normalizeSchema builds from each plan's includes. If a plan sets meteredBy to a feature that it does not include, no paykit_feature row exists. insertProductVersion then writes metered_feature_id and violates the foreign key added in packages/paykit/src/database/migrations/0004_add_metered_pricing.sql.
Validate the reference during normalization so the failure is a clear configuration error instead of a database error.
🛡️ Proposed validation
if (price) {
if (isMeteredPrice(price)) {
priceUsageType = "metered";
meteredFeatureId = price.meteredBy.id;
priceAmountDollars = price.unitAmount;
+ if (!includes.some((include) => include.id === meteredFeatureId)) {
+ throw new Error(
+ `Plan "${exportedPlan.id}" prices by metered feature "${meteredFeatureId}", which it does not include.`,
+ );
+ }
} else {📝 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.
| const price = exportedPlan.price; | |
| let priceUsageType: PriceUsageType = "licensed"; | |
| let meteredFeatureId: string | null = null; | |
| let priceAmountDollars: number | null = null; | |
| if (price) { | |
| if (isMeteredPrice(price)) { | |
| priceUsageType = "metered"; | |
| meteredFeatureId = price.meteredBy.id; | |
| priceAmountDollars = price.unitAmount; | |
| } else { | |
| priceAmountDollars = price.amount; | |
| } | |
| } | |
| const price = exportedPlan.price; | |
| let priceUsageType: PriceUsageType = "licensed"; | |
| let meteredFeatureId: string | null = null; | |
| let priceAmountDollars: number | null = null; | |
| if (price) { | |
| if (isMeteredPrice(price)) { | |
| priceUsageType = "metered"; | |
| meteredFeatureId = price.meteredBy.id; | |
| priceAmountDollars = price.unitAmount; | |
| if (!includes.some((include) => include.id === meteredFeatureId)) { | |
| throw new Error( | |
| `Plan "${exportedPlan.id}" prices by metered feature "${meteredFeatureId}", which it does not include.`, | |
| ); | |
| } | |
| } else { | |
| priceAmountDollars = price.amount; | |
| } | |
| } |
🤖 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 `@packages/paykit/src/types/schema.ts` around lines 445 - 457, Update the plan
normalization logic around metered price handling to verify that
price.meteredBy.id references a feature included by the same plan before
assigning meteredFeatureId. Reject invalid references with a clear configuration
error during normalizeSchema, while preserving the existing metered pricing
values for valid references.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 44 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/paykit/src/types/schema.ts">
<violation number="1" location="packages/paykit/src/types/schema.ts:452">
P1: A metered feature used only by `meteredBy` is never persisted before the product references it, so product sync fails its `metered_feature_id` foreign key. Register the referenced feature during normalization, or reject metered prices whose meter feature is not included.</violation>
</file>
<file name="packages/paykit/src/usage/usage.api.ts">
<violation number="1" location="packages/paykit/src/usage/usage.api.ts:7">
P3: `reportUsage` loses product-schema feature-ID inference, so TypeScript accepts misspelled or non-metered feature IDs and callers only discover the mistake at runtime. Add `reportUsage` to the same `FeatureIdFromOptions` refinement used by `check` and `report`.</violation>
</file>
<file name="packages/paykit/src/stripe/stripe-provider.ts">
<violation number="1" location="packages/paykit/src/stripe/stripe-provider.ts:272">
P2: Initial sync can issue multiple `meters.create` calls for one feature when multiple metered plans share `meterEventName`, instead of reusing one Billing Meter. Deduplicate or serialize `ensureStripeMeter` per event name for the duration of `syncProducts`.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| if (price) { | ||
| if (isMeteredPrice(price)) { | ||
| priceUsageType = "metered"; | ||
| meteredFeatureId = price.meteredBy.id; |
There was a problem hiding this comment.
P1: A metered feature used only by meteredBy is never persisted before the product references it, so product sync fails its metered_feature_id foreign key. Register the referenced feature during normalization, or reject metered prices whose meter feature is not included.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/types/schema.ts, line 452:
<comment>A metered feature used only by `meteredBy` is never persisted before the product references it, so product sync fails its `metered_feature_id` foreign key. Register the referenced feature during normalization, or reject metered prices whose meter feature is not included.</comment>
<file context>
@@ -422,17 +442,33 @@ export function normalizeSchema(
+ if (price) {
+ if (isMeteredPrice(price)) {
+ priceUsageType = "metered";
+ meteredFeatureId = price.meteredBy.id;
+ priceAmountDollars = price.unitAmount;
+ } else {
</file context>
|
|
||
| /** Finds or creates the Stripe Billing Meter whose `event_name` matches the PayKit feature id. */ | ||
| async function ensureStripeMeter(client: StripeSdk, eventName: string): Promise<string> { | ||
| const existing = await client.billing.meters.list({ status: "active" }); |
There was a problem hiding this comment.
P2: Initial sync can issue multiple meters.create calls for one feature when multiple metered plans share meterEventName, instead of reusing one Billing Meter. Deduplicate or serialize ensureStripeMeter per event name for the duration of syncProducts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/stripe/stripe-provider.ts, line 272:
<comment>Initial sync can issue multiple `meters.create` calls for one feature when multiple metered plans share `meterEventName`, instead of reusing one Billing Meter. Deduplicate or serialize `ensureStripeMeter` per event name for the duration of `syncProducts`.</comment>
<file context>
@@ -228,6 +257,34 @@ function isStripeResourceMissingError(error: unknown): boolean {
+
+/** Finds or creates the Stripe Billing Meter whose `event_name` matches the PayKit feature id. */
+async function ensureStripeMeter(client: StripeSdk, eventName: string): Promise<string> {
+ const existing = await client.billing.meters.list({ status: "active" });
+ const found = existing.data.find((meter) => meter.event_name === eventName);
+ if (found) {
</file context>
| import { reportUsageToProvider } from "./usage.service"; | ||
|
|
||
| const reportUsageBodySchema = z.object({ | ||
| featureId: z.string(), |
There was a problem hiding this comment.
P3: reportUsage loses product-schema feature-ID inference, so TypeScript accepts misspelled or non-metered feature IDs and callers only discover the mistake at runtime. Add reportUsage to the same FeatureIdFromOptions refinement used by check and report.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/usage/usage.api.ts, line 7:
<comment>`reportUsage` loses product-schema feature-ID inference, so TypeScript accepts misspelled or non-metered feature IDs and callers only discover the mistake at runtime. Add `reportUsage` to the same `FeatureIdFromOptions` refinement used by `check` and `report`.</comment>
<file context>
@@ -0,0 +1,25 @@
+import { reportUsageToProvider } from "./usage.service";
+
+const reportUsageBodySchema = z.object({
+ featureId: z.string(),
+ quantity: z.number().positive().optional(),
+ eventId: z.string().optional(),
</file context>
- Make addSubscriptionItem, removeSubscriptionItem, reportUsageEvent optional on PaymentProvider with a clear unsupported-provider error - Serialize add-on attachment with an advisory lock and tie the provider idempotency key to the pre-generated local row id - Validate targetSubscriptionId against the customer's active local rows - Paginate all subscription items in stripe provider flows and ensureStripeMeter - Require a meterEventName for metered products and reject empty checkout products - Omit quantity for metered prices in add/update subscription paths - Enforce metered unitAmount >= one cent and strict licensed/metered price schemas - Validate addOnPlanIds duplicates and length in the subscribe schema - Require priceUsageType metered in the usage eligibility query
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/paykit/src/subscription/subscription.service.ts (2)
1394-1407: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftLoad remove add-on from the stored row, not from group context.
removeSubscriptionAddOnreadssubCtx.activeSubscription, butloadSubscribeContextsets that tonullwhenstoredPlan.groupis falsy. That makes ungrouped add-ons fail withADDON_NOT_ACTIVEeven though removal uses the add-on’s local row (productId, anchor subscription item). Check the active row tied to thisinput.planIdfor both this function and the adjacent duplicate-attachment guard.🤖 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 `@packages/paykit/src/subscription/subscription.service.ts` around lines 1394 - 1407, The removeSubscriptionAddOn flow and its adjacent duplicate-attachment guard must use the active stored subscription row associated with input.planId instead of subCtx.activeSubscription, which may be null for ungrouped add-ons. Update both checks to read the local row’s stripe subscription and subscription-item identifiers while preserving the existing error behavior.
1301-1348: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the addon duplication guard on
productInternalId.
product.groupdefaults to"", and an add-on plan may live in its own group like"addons". Inside the transaction, the existing active subscription check branches onsubCtx.storedPlan.group, so an add-on plan with an empty group would skipgetActiveSubscriptionInGroup(tx, ...)and insert a duplicate provider subscription item. Query bysubCtx.storedPlan.internalIdinstead, matching the advisory lock scope.🤖 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 `@packages/paykit/src/subscription/subscription.service.ts` around lines 1301 - 1348, Update the active add-on duplication check in addSubscriptionAddOn to query by subCtx.storedPlan.internalId through getActiveSubscriptionInGroup, rather than branching on subCtx.storedPlan.group. Keep the lookup aligned with the advisory lock key so plans with empty or independent groups still detect an existing add-on.
🤖 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 `@packages/paykit/src/usage/usage.service.ts`:
- Around line 60-65: Update the reportUsageEvent invocation in the usage
reporting flow to preserve ctx.provider as its receiver. Keep the existing
unsupported-operation check, but call the method through
ctx.provider.reportUsageEvent or explicitly bind it before invocation so
implementations relying on this continue to work.
---
Outside diff comments:
In `@packages/paykit/src/subscription/subscription.service.ts`:
- Around line 1394-1407: The removeSubscriptionAddOn flow and its adjacent
duplicate-attachment guard must use the active stored subscription row
associated with input.planId instead of subCtx.activeSubscription, which may be
null for ungrouped add-ons. Update both checks to read the local row’s stripe
subscription and subscription-item identifiers while preserving the existing
error behavior.
- Around line 1301-1348: Update the active add-on duplication check in
addSubscriptionAddOn to query by subCtx.storedPlan.internalId through
getActiveSubscriptionInGroup, rather than branching on subCtx.storedPlan.group.
Keep the lookup aligned with the advisory lock key so plans with empty or
independent groups still detect an existing add-on.
🪄 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 Plus
Run ID: b6a6fb14-cc1b-4dcc-81e3-a80c69180d10
📒 Files selected for processing (14)
e2e/core/checkout/renewal-after-combined-checkout.test.tse2e/core/subscribe/downgrade-with-addon.test.tse2e/core/webhook/subscription-deleted-multi-item.test.tspackages/paykit/src/api/define-route.tspackages/paykit/src/core/errors.tspackages/paykit/src/product/__tests__/product.service.test.tspackages/paykit/src/product/product-sync.service.tspackages/paykit/src/product/product.service.tspackages/paykit/src/providers/provider.tspackages/paykit/src/stripe/stripe-provider.tspackages/paykit/src/subscription/subscription.service.tspackages/paykit/src/subscription/subscription.types.tspackages/paykit/src/types/schema.tspackages/paykit/src/usage/usage.service.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- e2e/core/webhook/subscription-deleted-multi-item.test.ts
- e2e/core/subscribe/downgrade-with-addon.test.ts
- e2e/core/checkout/renewal-after-combined-checkout.test.ts
- packages/paykit/src/product/tests/product.service.test.ts
- packages/paykit/src/product/product.service.ts
- packages/paykit/src/product/product-sync.service.ts
- packages/paykit/src/core/errors.ts
- packages/paykit/src/providers/provider.ts
- packages/paykit/src/types/schema.ts
- packages/paykit/src/stripe/stripe-provider.ts
| const reportUsageEvent = ctx.provider.reportUsageEvent; | ||
| if (!reportUsageEvent) { | ||
| throw PayKitError.from("BAD_REQUEST", PAYKIT_ERROR_CODES.PROVIDER_OPERATION_UNSUPPORTED); | ||
| } | ||
|
|
||
| const { providerEventId } = await reportUsageEvent({ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\breportUsageEvent\b|\bthis\.' \
packages/paykit/src/providers \
packages/paykit/src/stripe \
packages/paykit/src/usageRepository: getpaykit/paykit
Length of output: 4501
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching provider/usage/stripe:\n'
git ls-files | rg '(^|/)packages/paykit/src/(providers/provider\.ts|usage/usage\.service\.ts|stripe/stripe-provider\.ts)$|providers|usage|stripe' || true
printf '\nProvider type declarations around reportUsageEvent:\n'
sed -n '168,210p' packages/paykit/src/providers/provider.ts
printf '\nStripe provider implementation around reportUsageEvent:\n'
sed -n '1208,1255p' packages/paykit/src/stripe/stripe-provider.ts
printf '\nOther provider-like objects/callbacks using optional method shorthand:\n'
rg -n -C 6 'reportUsageEvent\?|reportUsageEvent\s*:|reportUsageEvent\s*=' packages/paykit/src/providers packages/paykit/src/stripe packages/paykit/src/usage || true
printf '\nTests mentioning custom provider/reportUsageEvent:\n'
rg -n -C 5 'reportUsageEvent|custom provider|ProviderOperationUnsupported|Provider' packages -g '*.test.*' -g '*.spec.*' || true
printf '\nRead-only JS probe: detached object method loses receiver but executes in loose mode;\nstrict functions throw this is undefined;\nif method uses only its data argument, no failure occurs\n'
node - <<'JS'
const strict = function() {'use strict'; void this};
const loose = function() { return this === undefined };
const strictArrow = () => void this;
console.log('strictFunctionThisIsUndefined', strict.call({}));
console.log('looseFunctionThisIsUndefined', loose.call({}));
console.log('strictArrowThisIsUndefined', strictArrow.call({}));
const handler = { reportUsageEvent(data) { return Object.keys(this).join('') + JSON.stringify(data) } };
const detached = handler.reportUsageEvent;
try {
console.log('detachedMethodLoose', detached({ foo: 1 }));
} catch (err) {
console.log('detachedMethodError', err.name + ': ' + err.message);
}
const handler2 = { reportUsageEvent(data) { 'use strict'; return this.foo } };
try {
console.log(JSON.stringify(handler2.reportUsageEvent({ foo: 1 })));
} catch (err) {
console.log('strictDetachedMethodError', err.name + ': ' + err.message);
}
JSRepository: getpaykit/paykit
Length of output: 19940
Preserve the provider receiver before calling reportUsageEvent.
ctx.provider.reportUsageEvent is detached on line 60 and invoked on line 65. A custom reportUsageEvent implementation that reads this loses its receiver; call it as ctx.provider.reportUsageEvent(...) or bind it explicitly.
🤖 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 `@packages/paykit/src/usage/usage.service.ts` around lines 60 - 65, Update the
reportUsageEvent invocation in the usage reporting flow to preserve ctx.provider
as its receiver. Keep the existing unsupported-operation check, but call the
method through ctx.provider.reportUsageEvent or explicitly bind it before
invocation so implementations relying on this continue to work.
There was a problem hiding this comment.
2 issues found across 14 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/paykit/src/subscription/subscription.service.ts">
<violation number="1" location="packages/paykit/src/subscription/subscription.service.ts:1334">
P1: A retry after a post-Stripe database failure can attach the add-on twice because this ID is regenerated for every invocation. Persist and reuse an operation ID before the provider call, or accept a stable caller request ID for the idempotency key.</violation>
</file>
<file name="packages/paykit/src/api/define-route.ts">
<violation number="1" location="packages/paykit/src/api/define-route.ts:329">
P2: Route creation fails for consumers resolved to a valid Zod 4.0.x release because `safeExtend` was added only in Zod 4.1. Raise PayKit's Zod minimum version to 4.1.0 (and refresh the lockfile), or avoid this API.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Serialize concurrent requests for the same customer/plan so only one can | ||
| // attach the add-on, and tie the provider item creation to a pre-generated | ||
| // local row id so a retried attempt cannot create duplicate provider items. | ||
| const addOnSubscriptionId = generateId("addon"); |
There was a problem hiding this comment.
P1: A retry after a post-Stripe database failure can attach the add-on twice because this ID is regenerated for every invocation. Persist and reuse an operation ID before the provider call, or accept a stable caller request ID for the idempotency key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/subscription/subscription.service.ts, line 1334:
<comment>A retry after a post-Stripe database failure can attach the add-on twice because this ID is regenerated for every invocation. Persist and reuse an operation ID before the provider call, or accept a stable caller request ID for the idempotency key.</comment>
<file context>
@@ -1323,32 +1323,59 @@ export async function addSubscriptionAddOn(
+ // Serialize concurrent requests for the same customer/plan so only one can
+ // attach the add-on, and tie the provider item creation to a pre-generated
+ // local row id so a retried attempt cannot create duplicate provider items.
+ const addOnSubscriptionId = generateId("addon");
+ const providerResult = await ctx.database.transaction(async (tx) => {
+ await tx.execute(
</file context>
| } | ||
|
|
||
| return Object.keys(overrides).length > 0 ? schema.extend(overrides) : schema; | ||
| return Object.keys(overrides).length > 0 ? schema.safeExtend(overrides) : schema; |
There was a problem hiding this comment.
P2: Route creation fails for consumers resolved to a valid Zod 4.0.x release because safeExtend was added only in Zod 4.1. Raise PayKit's Zod minimum version to 4.1.0 (and refresh the lockfile), or avoid this API.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/paykit/src/api/define-route.ts, line 329:
<comment>Route creation fails for consumers resolved to a valid Zod 4.0.x release because `safeExtend` was added only in Zod 4.1. Raise PayKit's Zod minimum version to 4.1.0 (and refresh the lockfile), or avoid this API.</comment>
<file context>
@@ -326,7 +326,7 @@ function createRouteInputSchema(schema: PayKitMethodConfig["input"]) {
}
- return Object.keys(overrides).length > 0 ? schema.extend(overrides) : schema;
+ return Object.keys(overrides).length > 0 ? schema.safeExtend(overrides) : schema;
}
</file context>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
19-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the repository’s declared Node version.
.github/workflows/release.ymluses.nvmrc, but this workflow hard-codes22. If.nvmrcdiffers, publishing uses a different runtime.actions/setup-nodesupportsnode-version-file. (github.com)Prefer this configuration, or verify that
.nvmrcis intentionally Node 22.Proposed fix
- node-version: 22 + node-version-file: .nvmrc🤖 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/publish.yml around lines 19 - 21, Update the setup-node configuration in the publish workflow to use the repository-declared Node version via node-version-file pointing to .nvmrc, matching the approach in the release workflow, instead of hard-coding node-version 22.
🤖 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/publish.yml:
- Around line 24-26: Update the publish workflow steps around npm ci, npm run
build, and npm publish so release commands execute from packages/paykit rather
than the private repository root. Prefer the repository’s existing
pnpm/Changesets release command; otherwise configure the package-specific
install and publish steps to use the pnpm-managed lockfile and publish the
paykit package only.
- Around line 17-19: Update the actions/checkout@v4 and actions/setup-node@v4
entries in the publishing workflow to immutable commit SHA pins, reusing the
corresponding SHA values already used by other workflows for these action
versions while preserving their current action versions and order.
- Line 17: Update the actions/checkout@v4 step in the publish workflow to
disable credential persistence by setting persist-credentials to false, without
changing the remaining checkout behavior.
- Around line 8-10: Update the npm publishing workflow around the publish step
to configure authentication by setting NODE_AUTH_TOKEN from secrets.NPM_TOKEN
and enabling NPM_CONFIG_PROVENANCE. Ensure these environment variables apply to
npm publish while preserving the existing permissions configuration.
---
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 19-21: Update the setup-node configuration in the publish workflow
to use the repository-declared Node version via node-version-file pointing to
.nvmrc, matching the approach in the release workflow, instead of hard-coding
node-version 22.
🪄 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 Plus
Run ID: 30f1181b-67ee-4cd0-9d59-283af3aea68d
📒 Files selected for processing (1)
.github/workflows/publish.yml
| permissions: | ||
| id-token: write | ||
| contents: read |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files .github/workflows | sed -n '1,80p'
printf '\npublish.yml:\n'
cat -n .github/workflows/publish.yml 2>/dev/null || true
printf '\nrelease.yml:\n'
cat -n .github/workflows/release.yml 2>/dev/null || true
printf '\nSearch npm publish/auth config:\n'
rg -n "npm publish|NODE_AUTH_TOKEN|NPM_TOKEN|npm_config_token|id-token|setup-node|actions/setup-node|NPM_CONFIG_PROVENANCE|registry-url" .github/workflows package.json packages 2>/dev/null || trueRepository: getpaykit/paykit
Length of output: 4133
🌐 Web query:
npm trusted publishing workflow actions setup-node id-token write npm CLI version Node version
💡 Result:
To use npm trusted publishing in GitHub Actions, you must configure a trusted publisher on npmjs.com [1][2] and update your workflow to use OIDC authentication [1][3]. Requirements: - npm CLI version: 11.5.1 or later is required [3][4][5]. - Node.js version: Node.js 24 or newer is recommended, as it includes a compatible version of npm by default [3][4][6]. Workflow Setup: 1. Permissions: You must grant the workflow permission to generate an OIDC token by setting id-token: write in your job or workflow definition [1][3][2]. 2. setup-node Configuration: When using actions/setup-node, you should include registry-url: 'https://registry.npmjs.org' [1][3]. 3. Avoiding Authentication Conflicts: A common issue is that actions/setup-node may write an _authToken placeholder to the .npmrc file if registry-url is specified, which can cause npm to fall back to legacy token-based authentication and fail [7][8]. To ensure the workflow correctly initiates the OIDC exchange, you may need to ensure NODE_AUTH_TOKEN is unset or empty during the publish step [9][8]. Alternatively, using recent versions of actions/setup-node or adding an explicit step to clear environment tokens before npm publish is recommended [9][8]. Example Workflow Snippet: permissions: contents: read id-token: write # Required for OIDC jobs: publish: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - uses: actions/setup-node@v6 with: node-version: '24' registry-url: 'https://registry.npmjs.org' - run: npm ci - run: npm publish # npm 11.5.1+ will automatically use OIDC [3][9] If you are unable to upgrade to Node.js 24, you can manually install the required npm version in your workflow before publishing: run: npm install -g npm@latest [4][10][6].
Citations:
- 1: https://docs.npmjs.com/trusted-publishers/
- 2: https://github.com/npm/documentation/blob/main/content/packages-and-modules/securing-your-code/trusted-publishers.mdx
- 3: actions/setup-node@670825a
- 4: https://jescalada.com/blog/2026-02-01-switching-npm-trusted-publishers-mind-node-version/
- 5: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/
- 6: https://projen.io/docs/publishing/trusted-publishing/
- 7: registry-url writes _authToken line that breaks npm Trusted Publisher OIDC when no NODE_AUTH_TOKEN is set actions/setup-node#1551
- 8: Trusted Publisher documentation actions/setup-node#1445
- 9: Don't default NPM_AUTH_TOKEN to support NPM OIDC actions/setup-node#1440
- 10: https://philna.sh/blog/2026/01/28/trusted-publishing-npm/
🌐 Web query:
npmjs.com trusted publishing setup publish npm publish authentication setup-node id-token node-version 22 npm 11.5.1
💡 Result:
To set up npm Trusted Publishing, which allows you to publish packages without storing long-lived npm tokens, follow these steps. ### Prerequisites * npm version 11.5.1 or higher [1]. * Node.js 22 or higher (Node.js 24 recommended) [1]. * Two-factor authentication (2FA) must be enabled on your npm account [2]. * Your package must already exist on the npm registry [2][3]. ### Step 1: Configure Trusted Publisher on npmjs.com 1. Log in to npmjs.com and navigate to your package settings [4][5]. 2. Locate the "Trusted Publisher" section and select your CI/CD provider (e.g., GitHub Actions) [4][5]. 3. Provide the required details: * GitHub: Owner/Organization, Repository name, and Workflow filename (e.g., publish.yml, without the file path) [4][5]. * Select the allowed actions: npm publish and/or npm stage publish [4][5]. 4. Save the configuration [6]. ### Step 2: Configure your GitHub Actions Workflow Update your workflow file (.github/workflows/publish.yml) to grant the necessary OIDC permissions [4][1]: yaml jobs: publish: runs-on: ubuntu-latest permissions: contents: read id-token: write # Required for OIDC authentication steps: - uses: actions/checkout@v6 - uses: actions/setup-node@v6 with: node-version: '24' registry-url: 'https://registry.npmjs.org' - run: npm ci - run: npm publish ### Important Authentication Notes * No Long-Lived Tokens: Do not set or provide an NPM_TOKEN or NODE_AUTH_TOKEN secret when using Trusted Publishing [3][7]. * setup-node Behavior: When registry-url is set, actions/setup-node may attempt to write an _authToken placeholder to your .npmrc [8]. If your publishing job fails with ENEEDAUTH or E404, you may need to explicitly prevent this by adding auth-token-line: false to your setup-node step [8]: yaml - uses: actions/setup-node@v6 with: node-version: '24' registry-url: 'https://registry.npmjs.org' auth-token-line: false * Provenance: When using Trusted Publishing, the npm CLI automatically generates provenance attestations for your package, so the --provenance flag is not required [6].
Citations:
- 1: actions/setup-node@670825a
- 2: https://docs.npmjs.com/cli/v11/commands/npm-trust/
- 3: https://circleci.com/docs/guides/deploy/deploy-to-npm-registry/
- 4: https://docs.npmjs.com/trusted-publishers/
- 5: https://github.com/npm/documentation/blob/main/content/packages-and-modules/securing-your-code/trusted-publishers.mdx
- 6: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/
- 7: https://safeguard.sh/resources/blog/npm-trusted-publishing-walkthrough-2026
- 8: registry-url writes _authToken line that breaks npm Trusted Publisher OIDC when no NODE_AUTH_TOKEN is set actions/setup-node#1551
🌐 Web query:
npmjs.com trusted publishing Node.js 22 npm CLI compatible version default installed by Node.js 22.14.0
💡 Result:
Trusted publishing on npm requires npm CLI version 11.5.1 or later and Node.js version 22.14.0 or higher [1][2]. While Node.js 22.14.0 is the minimum required Node.js version for this feature, it is not bundled with the necessary npm CLI version by default [1][2]. Node.js 22.14.0 comes with npm 10.9.2 [3], which is older than the required npm CLI version 11.5.1 [1][2]. To use trusted publishing, users must manually upgrade their npm CLI to version 11.5.1 or later [1][2].
Citations:
- 1: https://docs.npmjs.com/trusted-publishers/
- 2: https://circleci.com/docs/guides/deploy/deploy-to-npm-registry/
- 3: https://docs.aws.amazon.com/apprunner/latest/relnotes/release-2025-04-24-nodejs22-rt-release.html
Configure npm authentication for npm publish.
This workflow does not set NODE_AUTH_TOKEN, and trusted publishing is not ready from the workflow alone: Node 22 installs npm < 11.5.1 by default, so npm CLI 11.5.1+ must be installed explicitly with registry-url configured, or use the token path. For the existing token path, set NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} and NPM_CONFIG_PROVENANCE: true around npm publish.
[security And_privacy]
🤖 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/publish.yml around lines 8 - 10, Update the npm publishing
workflow around the publish step to configure authentication by setting
NODE_AUTH_TOKEN from secrets.NPM_TOKEN and enabling NPM_CONFIG_PROVENANCE.
Ensure these environment variables apply to npm publish while preserving the
existing permissions configuration.
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files '.github/workflows/*' || true
echo "== publish.yml excerpt =="
if [ -f .github/workflows/publish.yml ]; then
nl -ba .github/workflows/publish.yml | sed -n '1,220p'
fi
echo "== checkout usages in publish.yml with context =="
if [ -f .github/workflows/publish.yml ]; then
rg -n -C 6 'actions/checkout|persist-credentials|npm|pnpm|yarn|git ' .github/workflows/publish.yml || true
fi
echo "== deterministic docs check from installed action metadata not available; inspect local if present =="
(
tmp="$(mktemp -d)"
cd "$tmp"
if command -v git >/dev/null 2>&1; then
git clone --depth 1 --filter=blob:none --sparse https://github.com/actions/checkout.git 2>/dev/null || true
if [ -d checkout ]; then
cd checkout
rg -n "persist-credentials|token|auth" README.md dist/index.js src/ 2>/dev/null | head -n 80 || true
fi
fi
)Repository: getpaykit/paykit
Length of output: 387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== publish.yml relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' .github/workflows/publish.yml | sed -n '1,220p'
echo "== checkout usages in publish.yml =="
grep -n -C 6 'actions/checkout\|persist-credentials\|npm\|yarn\|pnpm\|git ' .github/workflows/publish.yml || true
echo "== all workflow checkout occurrences with inputs =="
grep -n -C 3 'actions/checkout\|persist-credentials' .github/workflows/*.yml || true
echo "== package script definitions mentioning publish/build =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
if p.exists():
data=json.loads(p.read_text())
for k,v in data.get('scripts',{}).items():
if any(s in v.lower() for s in ['npm publish','publish','build','npm pack','dist-tag','ci']):
print(f"{k}: {v}")
PYRepository: getpaykit/paykit
Length of output: 6584
🌐 Web query:
actions/checkout v4 persist-credentials default true README
💡 Result:
In actions/checkout v4, the persist-credentials input parameter defaults to true [1][2]. When this option is enabled (the default behavior), the authentication token or SSH key used for the checkout is persisted in the local git configuration [3][4]. This allows subsequent steps in your workflow job to execute authenticated git commands using those same credentials [3][4]. The token is automatically removed during the post-job cleanup phase [3][4]. To opt out of this behavior and prevent the credentials from being stored in the local git config, you must explicitly set persist-credentials to false in your workflow file [3][5]: - name: Checkout uses: actions/checkout@v4 with: persist-credentials: false While there has been ongoing community discussion regarding the security implications of this default [6][5], the official documentation and action configuration maintain true as the default setting for v4 [1][2].
Citations:
- 1: https://github.com/actions/checkout/blob/v4/action.yml
- 2: https://github.com/actions/checkout/blob/v4.1.1/action.yml
- 3: https://github.com/actions/checkout/blob/v4/README.md
- 4: https://github.com/marketplace/actions/checkout
- 5: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 6: [security] Escalate concerning default
persist-credentials=trueplease actions/checkout#2312
Disable checkout credential persistence.
actions/checkout@v4 persists the GitHub token by default. The publish job only needs checkout, setup, install, build, and publish steps, so explicit Git auth is unnecessary.
Proposed fix
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 17-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/publish.yml at line 17, Update the actions/checkout@v4
step in the publish workflow to disable credential persistence by setting
persist-credentials to false, without changing the remaining checkout behavior.
Source: Linters/SAST tools
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files '.github/workflows/*' | sort
echo
echo "== publish workflow relevant lines =="
cat -n .github/workflows/publish.yml | sed -n '1,80p'
echo
echo "== release workflow relevant action lines =="
if [ -f .github/workflows/release.yml ]; then
cat -n .github/workflows/release.yml | rg -n "uses:" -C 2
else
echo "release.yml not found"
fi
echo
echo "== all uses tags/shas in workflows =="
rg -n 'uses:\s*[^#]+' .github/workflows || trueRepository: getpaykit/paykit
Length of output: 4865
🌐 Web query:
zizmor audit hash pins sensitive workflows .github/workflows docs.zizmor.sh audits
💡 Result:
Zizmor is a static analysis tool for GitHub Actions workflows that includes specific audit rules to enforce secure dependency pinning [1][2]. A primary audit rule related to this is unpinned-uses, which checks for workflows that do not use immutable SHA-based references for their actions [3][4]. Key aspects of zizmor auditing and hash pinning include: Hash-Pinning Enforcement: The unpinned-uses audit detects when actions are referenced by tags, branches, or other non-immutable references [3]. Since version 1.20.0, zizmor defaults to a strict policy requiring all actions to be pinned by SHA reference (hash-pinning) [3][4]. This prevents supply chain attacks where a malicious actor could overwrite a tag or branch reference with compromised code [5]. Sensitive Workflows and Audits: Zizmor audits help identify risks in workflows, such as excessive permissions or potential credential leakage [6]. Because hash-pinning creates a permanent link to a specific commit, it is a recommended practice for all sensitive or critical workflows to ensure dependency integrity [3][5]. Related Audit Checks: - Version Comment Mismatch: Zizmor audits hash-pinned actions to ensure their associated version comments (often used by tools like Dependabot) are accurate and match the pinned commit [3][7]. - Stale Action Refs: The stale-action-refs audit flags SHA references that do not point to a known Git tag, as these can obscure the version history and release notes associated with the action [7]. Configuration: While the default policy is strict, users can adjust these behaviors in their zizmor.yml configuration file [3][4]. For example, users can restore a more permissive policy for specific action namespaces (like actions/* or github/*) if they prefer ref-pinning for those repositories [4]. Online vs. Offline Mode: While the audit rules themselves can often be run offline, zizmor's fixing capabilities—such as automatically generating the correct SHA-pin for a reference—require online access to fetch commit history from the GitHub API [8][9]. When handling sensitive or private repositories, users should be aware that zizmor caches HTTP responses locally, and they should exercise caution when sharing these caches [8].
Citations:
- 1: https://zizmor.sh/
- 2: https://docs.zizmor.sh/
- 3: https://docs.zizmor.sh/audits/
- 4: https://github.com/zizmorcore/zizmor/releases/tag/v1.20.0
- 5: Reconsider the default inclusion of official GitHub actions in unpinned-uses zizmorcore/zizmor#1434
- 6: https://github.com/zizmorcore/zizmor
- 7: https://github.com/zizmorcore/zizmor/blob/563b7b25/docs/audits.md
- 8: https://github.com/zizmorcore/zizmor/blob/main/docs/usage.md
- 9: https://docs.zizmor.sh/usage/
🌐 Web query:
actions/checkout v4 commit SHA 34e114876b0b11c390a56381ad16ebd13914f8d5
💡 Result:
The commit SHA 34e114876b0b11c390a56381ad16ebd13914f8d5 corresponds to version v4.3.1 of the actions/checkout GitHub Action [1]. Pinning actions to specific commit SHAs is a security best practice to prevent unexpected behavior or malicious code execution if a tag (such as @v4) is updated by the repository maintainers [1]. This specific SHA has been widely used by developers to lock their workflows to the v4.3.1 release [2][1]. As with all versions of actions/checkout, it is used to check out your repository under the GITHUB_WORKSPACE environment, allowing your workflow to access the source code [3][4].
Citations:
- 1: dfinity/vetkeys@d5b39d4
- 2: https://github.com/LycaonLLC/t4-code/blob/6800c16e1e79b5090d1ed652137674315f27d10f/.github/workflows/ci.yml
- 3: https://github.com/actions/checkout
- 4: https://github.com/actions/checkout/blob/v4/README.md
🌐 Web query:
actions/setup-node v4 commit SHA 49933ea5288caeca8642d1e84afbd3f7d6820020
💡 Result:
The commit SHA 49933ea5288caeca8642d1e84afbd3f7d6820020 corresponds to version v4.4.0 of the actions/setup-node GitHub Action [1][2]. This version introduced support for private mirrors [3][4]. Pinning to this specific commit is a common practice in GitHub Actions workflows to ensure consistent, immutable behavior [1][2].
Citations:
- 1: Scan-Documents/node-sdk@fd4202b
- 2: Environment details is empty and node is not found actions/setup-node#1210
- 3: actions/setup-node@v4.2.0...49933ea
- 4: https://gitea.rs.h3.ink/actions/setup-node/src/commit/49933ea5288caeca8642d1e84afbd3f7d6820020/action.yml
Pin publishing actions to immutable commit SHAs.
This workflow grants id-token: write before publishing, so use permanent action pins instead of movable major-version tags. Use the SHAs already used by other workflows for these same action versions.
Proposed fix
- - uses: actions/checkout@v4
+ - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
...
- - uses: actions/setup-node@v4
+ - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/setup-node@v4 | |
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 | |
| - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 17-17: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 19-19: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 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/publish.yml around lines 17 - 19, Update the
actions/checkout@v4 and actions/setup-node@v4 entries in the publishing workflow
to immutable commit SHA pins, reusing the corresponding SHA values already used
by other workflows for these action versions while preserving their current
action versions and order.
| - run: npm ci | ||
| - run: npm run build --if-present | ||
| - run: npm publish |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked workflow/package files:"
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|\.github/workflows/publish\.yml|\.github/workflows/release\.yml)$|package\.json$|pnpm-lock\.yaml$' | sort
echo
echo "publish.yml:"
if [ -f .github/workflows/publish.yml ]; then
cat -n .github/workflows/publish.yml
fi
echo
echo "release.yml:"
if [ -f .github/workflows/release.yml ]; then
cat -n .github/workflows/release.yml
fi
echo
echo "Root package.json first 220 lines:"
if [ -f package.json ]; then
sed -n '1,220p' package.json | cat -n
fi
echo
echo "workspace members and paykit package.json:"
if [ -f packages/paykit/package.json ]; then
sed -n '1,220p' packages/paykit/package.json | cat -n
fi
echo
echo "package manager declarations:"
python3 - <<'PY'
import json, pathlib
for p in [pathlib.Path('package.json'), pathlib.Path('packages/paykit/package.json')]:
if p.exists():
data=json.loads(p.read_text())
print(f'{p}:')
for k in ['scripts','packageManager','private','publishConfig','workspaces','scripts']:
if k in data:
print(f' {k}={data[k]!r}')
PYRepository: getpaykit/paykit
Length of output: 8778
Do not publish from the root package using npm.
Run npm ci and npm publish from packages/paykit, or use the existing pnpm/Changesets release command. The root package is private, the repository uses pnpm, and this workflow can install from no npm lockfile or try to publish the wrong package.
🤖 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/publish.yml around lines 24 - 26, Update the publish
workflow steps around npm ci, npm run build, and npm publish so release commands
execute from packages/paykit rather than the private repository root. Prefer the
repository’s existing pnpm/Changesets release command; otherwise configure the
package-specific install and publish steps to use the pnpm-managed lockfile and
publish the paykit package only.
There was a problem hiding this comment.
5 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/publish.yml">
<violation number="1" location=".github/workflows/publish.yml:17">
P2: `actions/checkout@v4` persists the GitHub token in the local git config by default. Since this job only checks out, builds, and publishes, add `persist-credentials: false` to avoid leaving credentials accessible to later steps.</violation>
<violation number="2" location=".github/workflows/publish.yml:19">
P2: This workflow grants `id-token: write` and runs `npm publish`, but pins `actions/checkout` and `actions/setup-node` to mutable major-version tags (`@v4`) rather than immutable commit SHAs. Pin these actions to specific commit SHAs to reduce supply-chain risk during publishing.</violation>
<violation number="3" location=".github/workflows/publish.yml:24">
P0: The `npm ci` step will fail on this repository: it is a pnpm workspace (packageManager pnpm@11.1.1) and there is a pnpm-lock.yaml but no package-lock.json, which `npm ci` requires. This workflow will error out before it ever reaches the build or publish steps. Use the pnpm toolchain, consistent with the rest of the repo (ci.yml / release.yml): `pnpm/action-setup` followed by `pnpm install --frozen-lockfile`.</violation>
<violation number="4" location=".github/workflows/publish.yml:26">
P1: The `npm publish` step has no `NODE_AUTH_TOKEN` set in its environment, so npm cannot authenticate to the registry (setup-node's registry-url writes an .npmrc that expects that variable) and publish will fail with a 401. Add `env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}` to the publish step, matching the existing release.yml and npm-dist-tag.yml workflows.</violation>
<violation number="5" location=".github/workflows/publish.yml:26">
P0: Running `npm publish` from the repo root will be rejected because the root package.json is marked `private: true` (npm errors with "This package has been marked as private"). This workflow should follow the repo's established publish flow rather than publishing the root package: build the packages and invoke `scripts/publish-dist.mjs` (or run the `ci:release` script), which publishes `packages/paykit/dist` with `--access public`. As written, the publish step cannot succeed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| - run: npm ci | ||
| - run: npm run build --if-present | ||
| - run: npm publish |
There was a problem hiding this comment.
P0: Running npm publish from the repo root will be rejected because the root package.json is marked private: true (npm errors with "This package has been marked as private"). This workflow should follow the repo's established publish flow rather than publishing the root package: build the packages and invoke scripts/publish-dist.mjs (or run the ci:release script), which publishes packages/paykit/dist with --access public. As written, the publish step cannot succeed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 26:
<comment>Running `npm publish` from the repo root will be rejected because the root package.json is marked `private: true` (npm errors with "This package has been marked as private"). This workflow should follow the repo's established publish flow rather than publishing the root package: build the packages and invoke `scripts/publish-dist.mjs` (or run the `ci:release` script), which publishes `packages/paykit/dist` with `--access public`. As written, the publish step cannot succeed.</comment>
<file context>
@@ -0,0 +1,26 @@
+
+ - run: npm ci
+ - run: npm run build --if-present
+ - run: npm publish
</file context>
| node-version: 22 | ||
| registry-url: https://registry.npmjs.org | ||
|
|
||
| - run: npm ci |
There was a problem hiding this comment.
P0: The npm ci step will fail on this repository: it is a pnpm workspace (packageManager pnpm@11.1.1) and there is a pnpm-lock.yaml but no package-lock.json, which npm ci requires. This workflow will error out before it ever reaches the build or publish steps. Use the pnpm toolchain, consistent with the rest of the repo (ci.yml / release.yml): pnpm/action-setup followed by pnpm install --frozen-lockfile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 24:
<comment>The `npm ci` step will fail on this repository: it is a pnpm workspace (packageManager pnpm@11.1.1) and there is a pnpm-lock.yaml but no package-lock.json, which `npm ci` requires. This workflow will error out before it ever reaches the build or publish steps. Use the pnpm toolchain, consistent with the rest of the repo (ci.yml / release.yml): `pnpm/action-setup` followed by `pnpm install --frozen-lockfile`.</comment>
<file context>
@@ -0,0 +1,26 @@
+ node-version: 22
+ registry-url: https://registry.npmjs.org
+
+ - run: npm ci
+ - run: npm run build --if-present
+ - run: npm publish
</file context>
|
|
||
| - run: npm ci | ||
| - run: npm run build --if-present | ||
| - run: npm publish |
There was a problem hiding this comment.
P1: The npm publish step has no NODE_AUTH_TOKEN set in its environment, so npm cannot authenticate to the registry (setup-node's registry-url writes an .npmrc that expects that variable) and publish will fail with a 401. Add env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} to the publish step, matching the existing release.yml and npm-dist-tag.yml workflows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 26:
<comment>The `npm publish` step has no `NODE_AUTH_TOKEN` set in its environment, so npm cannot authenticate to the registry (setup-node's registry-url writes an .npmrc that expects that variable) and publish will fail with a 401. Add `env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}` to the publish step, matching the existing release.yml and npm-dist-tag.yml workflows.</comment>
<file context>
@@ -0,0 +1,26 @@
+
+ - run: npm ci
+ - run: npm run build --if-present
+ - run: npm publish
</file context>
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
P2: actions/checkout@v4 persists the GitHub token in the local git config by default. Since this job only checks out, builds, and publishes, add persist-credentials: false to avoid leaving credentials accessible to later steps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 17:
<comment>`actions/checkout@v4` persists the GitHub token in the local git config by default. Since this job only checks out, builds, and publishes, add `persist-credentials: false` to avoid leaving credentials accessible to later steps.</comment>
<file context>
@@ -0,0 +1,26 @@
+ runs-on: ubuntu-latest
+
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-node@v4
</file context>
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
P2: This workflow grants id-token: write and runs npm publish, but pins actions/checkout and actions/setup-node to mutable major-version tags (@v4) rather than immutable commit SHAs. Pin these actions to specific commit SHAs to reduce supply-chain risk during publishing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/publish.yml, line 19:
<comment>This workflow grants `id-token: write` and runs `npm publish`, but pins `actions/checkout` and `actions/setup-node` to mutable major-version tags (`@v4`) rather than immutable commit SHAs. Pin these actions to specific commit SHAs to reduce supply-chain risk during publishing.</comment>
<file context>
@@ -0,0 +1,26 @@
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-node@v4
+ with:
+ node-version: 22
</file context>
| - uses: actions/setup-node@v4 | |
| - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 |
1 participant
Summary
subscribe()with optionaladdOnPlanIdsso a base plan and add-ons share one Stripe Checkout session / subscriptionSummary by cubic
Add combined checkout for base plans and add-ons under one Stripe subscription and payment. Adds metered usage billing with usage reporting, hardens multi-item flows for reliability, and adds a tagged-release publish workflow.
New Features
subscribe()supportsaddOnPlanIdsto combine base + add-ons into one Stripe Checkout/subscription.price: { meteredBy, unitAmount, interval }and report usage viareportUsage().addAddOn()andremoveAddOn()to attach/detach add-ons on active subscriptions.addOnPlanIds(max 10); serialize add-on attach with an advisory lock and idempotency; paginate Stripe subscription items; reject empty checkout products; omit quantity for metered prices; enforceunitAmount >= $0.01; require metered products in usage checks; surface clearPROVIDER_OPERATION_UNSUPPORTEDwhen provider doesn’t implement optional add/remove/report operations.Migration
paykitjs pushto apply0003_add_subscription_item_idand0004_add_metered_pricing.createSubscriptionCheckoutto acceptproviderProducts(array). Implement (or explicitly not support)addSubscriptionItem,removeSubscriptionItem, andreportUsageEvent; unsupported operations will throw a clear error.Written for commit 80c499a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores