feat(billing): add fixed-threshold auto top-up API behind a feature flag - #3533
feat(billing): add fixed-threshold auto top-up API behind a feature flag#3533baktun14 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds persisted wallet auto-reload threshold and amount settings. It exposes validated API contracts, centralizes the $20 minimum, adds fixed-threshold reload execution behind a feature flag, updates instrumentation, and adjusts reload scheduling. ChangesAuto top-up configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/api/src/billing/lib/currency/currency.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/api/src/billing/lib/currency/currency.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3533 +/- ##
==========================================
- Coverage 75.08% 74.29% -0.79%
==========================================
Files 1162 1074 -88
Lines 30161 27852 -2309
Branches 7502 7058 -444
==========================================
- Hits 22645 20693 -1952
+ Misses 6644 6320 -324
+ Partials 872 839 -33
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop
#MIN_RELOAD_AMOUNT_IN_USDin favor of the newly imported shared constant.Line 53 still hardcodes
20while the fixed-threshold path usesSTANDARD_TOP_UP_MIN_AMOUNT_USDimported on line 6 — the two minimums can now drift, which is the exact duplicationtop-up.config.tswas added to eliminate.♻️ Proposed change
- `#MIN_RELOAD_AMOUNT_IN_USD` = 20; -- const reloadAmountInFiat = Math.max(costUntilTargetDateInFiat - resources.balance, this.#MIN_RELOAD_AMOUNT_IN_USD); + const reloadAmountInFiat = Math.max(costUntilTargetDateInFiat - resources.balance, STANDARD_TOP_UP_MIN_AMOUNT_USD);Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts` at line 6, Replace the hardcoded 20 in the wallet balance reload check with the imported STANDARD_TOP_UP_MIN_AMOUNT_USD constant, and remove the obsolete `#MIN_RELOAD_AMOUNT_IN_USD` definition if present. Keep both threshold paths aligned through the shared constant.apps/deploy-web/src/components/billing-usage/AutoTopUpSettingsPopup/AutoTopUpSettingsPopup.spec.tsx (1)
127-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNested object passed as
mock<T>()override.
mock<PaymentMethod>({ card: {...} })passes a plain nested object as an override. Based on learnings, this pattern can wrapcardin a recursive proxy, causing any of its unset sub-properties to resolve to mock functions instead ofundefined— assignpaymentMethod.card = {...}after creating the mock instead.🔧 Proposed fix
- const paymentMethod = mock<PaymentMethod>({ - card: { brand: "visa", last4: "5720", exp_month: 5, exp_year: 30 } as PaymentMethod["card"] - }); + const paymentMethod = mock<PaymentMethod>(); + paymentMethod.card = { brand: "visa", last4: "5720", exp_month: 5, exp_year: 30 } as PaymentMethod["card"];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/deploy-web/src/components/billing-usage/AutoTopUpSettingsPopup/AutoTopUpSettingsPopup.spec.tsx` around lines 127 - 129, Update the paymentMethod setup in the test to create the PaymentMethod mock without the nested card override, then assign the plain card object directly to paymentMethod.card afterward. Preserve the existing card brand, last4, exp_month, and exp_year values while avoiding recursive proxying of nested properties.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/billing/http-schemas/wallet.schema.ts`:
- Around line 53-71: Update WalletSettingsInputSchema to add explicit
upper-bound validation to both autoReloadAmount and autoReloadThreshold, using
maximums compatible with the database numeric(20, 2) range and safe charging
behavior in `#tryToReloadOnFixedThreshold`. Keep the existing minimum, precision,
optionality, and OpenAPI descriptions intact so oversized requests are rejected
during schema validation.
In
`@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts`:
- Around line 181-213: Update `#tryToReloadOnFixedThreshold` to evaluate
AUTO_RELOAD_FIXED_THRESHOLD using the job payload’s real user ID rather than the
worker’s CURRENT_USER value; pass payload.userId into the flag context used by
this reload decision while preserving the existing threshold and reload
behavior.
---
Nitpick comments:
In
`@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts`:
- Line 6: Replace the hardcoded 20 in the wallet balance reload check with the
imported STANDARD_TOP_UP_MIN_AMOUNT_USD constant, and remove the obsolete
`#MIN_RELOAD_AMOUNT_IN_USD` definition if present. Keep both threshold paths
aligned through the shared constant.
In
`@apps/deploy-web/src/components/billing-usage/AutoTopUpSettingsPopup/AutoTopUpSettingsPopup.spec.tsx`:
- Around line 127-129: Update the paymentMethod setup in the test to create the
PaymentMethod mock without the nested card override, then assign the plain card
object directly to paymentMethod.card afterward. Preserve the existing card
brand, last4, exp_month, and exp_year values while avoiding recursive proxying
of nested properties.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e886d2b-b272-4a30-b4a3-b9a828cf6f0a
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (28)
apps/api/drizzle/0034_cheerful_electro.sqlapps/api/drizzle/meta/0034_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/config/index.tsapps/api/src/billing/config/top-up.config.tsapps/api/src/billing/controllers/wallet-settings/wallet-settings.controller.spec.tsapps/api/src/billing/http-schemas/wallet.schema.tsapps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.tsapps/api/src/billing/repositories/wallet-settings/wallet-settings.repository.tsapps/api/src/billing/services/trial-validation/trial-validation.service.tsapps/api/src/billing/services/wallet-balance-reload-check/README.mdapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.spec.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.integration.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.tsapps/api/src/core/services/feature-flags/feature-flags.tsapps/api/swagger/openapi.jsonapps/api/test/functional/wallet-settings.spec.tsapps/api/test/seeders/wallet-setting.seeder.tsapps/deploy-web/src/components/billing-usage/AccountOverview/AccountOverview.spec.tsxapps/deploy-web/src/components/billing-usage/AccountOverview/AccountOverview.tsxapps/deploy-web/src/components/billing-usage/AutoTopUpSettingsPopup/AutoTopUpSettingsPopup.spec.tsxapps/deploy-web/src/components/billing-usage/AutoTopUpSettingsPopup/AutoTopUpSettingsPopup.tsxapps/deploy-web/src/queries/useWalletSettingsQueries.spec.tsxapps/deploy-web/src/types/feature-flags.tspackages/console-api-types/src/schema.d.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
b0def8a to
53725b0
Compare
baktun14
changed the title
| @@ -112,11 +112,24 @@ export class WalletSettingService { | |||
| } | |||
|
|
|||
| async #arrangeSchedule(prev?: WalletSettingOutput, next?: WalletSettingOutput) { | |||
There was a problem hiding this comment.
question: I don't understand the change. when is wallet reload job triggered after this change?
before this it was triggered based on estimation of lifetime for deployments user has or in some particular moments. Does this PR changes this?
There was a problem hiding this comment.
@stalniy, this PR changes both the reload-check scheduling and, behind the feature flag, the reload decision.
WalletSettingService.#arrangeSchedule() now schedules an immediate reload-check job in these cases:
-
autoReloadEnabledchanges from disabled to enabled.
It callsscheduleForWalletSetting(next, { withCleanup: true }). -
autoReloadEnabledstays enabled, and eitherautoReloadThresholdorautoReloadAmountchanges.
It callsscheduleForWalletSetting(next).
It does not schedule a job when auto reload is disabled or when enabled settings do not change.
The existing event-driven and periodic reload checks still schedule jobs. The change is what the handler does when a job runs:
- With
auto_reload_fixed_thresholddisabled, it keeps the existing predicted-spend algorithm based on deployment cost/lifetime. - With
auto_reload_fixed_thresholdenabled, it does not calculate predicted deployment spend. It reloads whenbalance <= autoReloadThreshold, and chargesautoReloadAmount(clamped to at least$20). - The fixed-threshold path can reload even when the user has no active deployments.
So, this service adds an immediate check after enabling or changing fixed-threshold settings. The feature flag changes the reload criterion from predicted deployment spend to the configured wallet balance threshold.
You are interacting with an AI system.
There was a problem hiding this comment.
The check scheduling is unchanged: the event-driven and periodic reload checks still enqueue jobs the same way. What this PR adds is an immediate check when a user enables auto top-up or edits their threshold or amount. With the auto_reload_fixed_threshold flag off, the predicted-lifetime estimation still runs; with it on, the decision becomes a plain balance-versus-threshold comparison that charges the configured amount even when there are no active deployments. So the cadence stays the same and only the reload criterion changes behind the flag.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Backend half of CON-717, split out from the combined PR. Adds the auto_reload_threshold/auto_reload_amount columns, the wallet-settings input/output schema split, the flag-gated reload-check branch, and regenerated OpenAPI + shared types. Everything is gated behind the auto_reload_fixed_threshold flag (off by default), so this is safe to merge ahead of the UI. Refs CON-717
53725b0 to
e8f0db4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts (2)
427-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the centralized minimum constant.
The test hardcodes
20for both the input and the clamped result. The minimum top-up amount now lives inapps/api/src/billing/config/top-up.config.ts. ImportSTANDARD_TOP_UP_MIN_AMOUNT_USDand derive the values from it, so a change to the constant fails this test instead of silently passing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts` around lines 427 - 438, Update the test around the setup call and createPaymentIntent assertion to import and use STANDARD_TOP_UP_MIN_AMOUNT_USD from the top-up configuration, deriving both autoReloadThreshold and the expected payment amount from that constant instead of hardcoding 20.
520-521: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBind the flag mock to the flag name.
isEnabledreturns the same value for every flag key. If the handler reads a second flag later, these tests keep passing for the wrong reason. Assert on the key instead.♻️ Proposed change
const featureFlagsService = mock<FeatureFlagsService>(); - featureFlagsService.isEnabled.mockReturnValue(input?.fixedThresholdEnabled ?? false); + featureFlagsService.isEnabled.mockImplementation(flag => flag === FeatureFlagValue.auto_reload_fixed_threshold && (input?.fixedThresholdEnabled ?? false));Adjust the flag identifier to match the enum exported from
apps/api/src/core/services/feature-flags/feature-flags.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts` around lines 520 - 521, Update the FeatureFlagsService mock setup around isEnabled in the wallet balance reload check tests to return the configured value only when called with the specific enum flag exported from feature-flags.ts, rather than for every flag key; preserve the existing false default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/billing/services/wallet-balance-reload-check/README.md`:
- Around line 21-25: Update the fenced pseudocode block in the README to specify
the text language identifier, changing the opening fence around the balance
auto-reload example to use text while preserving its contents.
---
Nitpick comments:
In
`@apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts`:
- Around line 427-438: Update the test around the setup call and
createPaymentIntent assertion to import and use STANDARD_TOP_UP_MIN_AMOUNT_USD
from the top-up configuration, deriving both autoReloadThreshold and the
expected payment amount from that constant instead of hardcoding 20.
- Around line 520-521: Update the FeatureFlagsService mock setup around
isEnabled in the wallet balance reload check tests to return the configured
value only when called with the specific enum flag exported from
feature-flags.ts, rather than for every flag key; preserve the existing false
default.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47d4fb9d-c822-4cef-b727-ff1434bbc0a5
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (22)
apps/api/drizzle/0035_cheerful_electro.sqlapps/api/drizzle/meta/0035_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/config/index.tsapps/api/src/billing/config/top-up.config.tsapps/api/src/billing/controllers/wallet-settings/wallet-settings.controller.spec.tsapps/api/src/billing/http-schemas/wallet.schema.tsapps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.tsapps/api/src/billing/repositories/wallet-settings/wallet-settings.repository.tsapps/api/src/billing/services/trial-validation/trial-validation.service.tsapps/api/src/billing/services/wallet-balance-reload-check/README.mdapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.spec.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.integration.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.tsapps/api/src/core/services/feature-flags/feature-flags.tsapps/api/swagger/openapi.jsonapps/api/test/functional/wallet-settings.spec.tsapps/api/test/seeders/wallet-setting.seeder.tspackages/console-api-types/src/schema.d.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/api/src/billing/config/index.ts
- apps/api/test/seeders/wallet-setting.seeder.ts
- apps/api/src/billing/config/top-up.config.ts
- apps/api/src/billing/services/trial-validation/trial-validation.service.ts
- apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.spec.ts
- apps/api/test/functional/wallet-settings.spec.ts
- apps/api/src/billing/repositories/wallet-settings/wallet-settings.repository.ts
- apps/api/src/core/services/feature-flags/feature-flags.ts
- apps/api/swagger/openapi.json
- apps/api/src/billing/controllers/wallet-settings/wallet-settings.controller.spec.ts
- apps/api/src/billing/http-schemas/wallet.schema.ts
- apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts
- apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts
- packages/console-api-types/src/schema.d.ts
- apps/api/src/billing/services/wallet-settings/wallet-settings.service.integration.ts
- apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts
- apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check-instrumentation.service.ts
…cents Persist auto_reload_threshold/auto_reload_amount as integer minor units instead of numeric(20,2) mapped to a JS float, matching the stripe_transaction.amount convention and removing any risk of float precision loss. The service converts dollars to cents on write and back on read; the reload handler converts to USD before comparing against the balance and charging. Also add the missing markdown code-fence language.
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 (1)
apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (1)
122-128: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake wallet-reload cleanup transaction-bound.
enqueueuses the active transaction, butcancelCreatedByuses a separatepgBossconnection. IfupsertWalletSettingrolls back, the cancellation remains while the new job enqueue rolls back, leaving no reload job. Run cleanup on the same transaction connection or defer it until commit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts` around lines 122 - 128, Update the wallet reload scheduling flow around `#hasReloadValuesChanged` and walletReloadJobService calls so cleanup uses the active transaction connection, matching enqueue behavior. Ensure cancellation is transaction-bound or deferred until commit, preventing cleanup from persisting when upsertWalletSetting rolls back and preserving the existing scheduling behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts`:
- Line 61: Update the private `#validate` method in upsertWalletSetting to enforce
the wallet-setting minimums for every provided value, independently of the
autoReloadEnabled branch, before either updateById or create persists settings.
Reuse the existing minimum-validation rules or symbols, and preserve the current
autoReloadEnabled-specific validation.
---
Outside diff comments:
In `@apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts`:
- Around line 122-128: Update the wallet reload scheduling flow around
`#hasReloadValuesChanged` and walletReloadJobService calls so cleanup uses the
active transaction connection, matching enqueue behavior. Ensure cancellation is
transaction-bound or deferred until commit, preventing cleanup from persisting
when upsertWalletSetting rolls back and preserving the existing scheduling
behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b48c8aa0-be94-4c74-9f22-45d284aca5c6
📒 Files selected for processing (12)
apps/api/drizzle/0035_cheerful_electro.sqlapps/api/drizzle/meta/0035_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/billing/lib/currency/currency.spec.tsapps/api/src/billing/lib/currency/currency.tsapps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.tsapps/api/src/billing/services/wallet-balance-reload-check/README.mdapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.tsapps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.integration.tsapps/api/src/billing/services/wallet-settings/wallet-settings.service.tsapps/api/test/seeders/wallet-setting.seeder.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/drizzle/0035_cheerful_electro.sql
- apps/api/drizzle/meta/_journal.json
- apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.spec.ts
- apps/api/src/billing/services/wallet-balance-reload-check/README.md
- apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts
|
|
||
| await this.#validate({ next: settings, userId }); | ||
| const next = await this.walletSettingRepository.accessibleBy(ability, "update").updateById(prev.id, settings, { returning: true }); | ||
| const next = await this.walletSettingRepository.accessibleBy(ability, "update").updateById(prev.id, this.#toStoredSettings(settings), { returning: true }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a -t f 'CLAUDE.md' . -x sh -c 'printf "\n--- %s ---\n" "$1"; sed -n "1,240p" "$1"' sh || true
rg -n -C 12 \
'upsertWalletSetting|autoReloadThreshold|autoReloadAmount' \
apps/api --glob '*.ts' --glob '*.tsx'Repository: akash-network/console
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- service ---'
sed -n '1,175p' apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts
printf '%s\n' '--- input schema and constants ---'
sed -n '1,115p' apps/api/src/billing/http-schemas/wallet.schema.ts
rg -n -C 3 'AUTO_RELOAD_THRESHOLD_MIN_USD|STANDARD_TOP_UP_MIN_AMOUNT_USD|AUTO_RELOAD_AMOUNT_MAX_USD' apps/api/src
printf '%s\n' '--- all upsert callers ---'
rg -n -C 4 'upsertWalletSetting' apps/api/src apps/api/test --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- repository write definitions ---'
rg -n -C 8 'class WalletSettingRepository|updateById|create\(' apps/api/src/billing/repositories apps/api/src/core/repositories --glob '*.ts'Repository: akash-network/console
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused service methods ---'
sed -n '35,125p' apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts
printf '%s\n' '--- schema imports and bounds ---'
sed -n '1,90p' apps/api/src/billing/http-schemas/wallet.schema.ts
rg -n -C 5 'AUTO_RELOAD_THRESHOLD_MIN_USD|STANDARD_TOP_UP_MIN_AMOUNT_USD|AUTO_RELOAD_AMOUNT_MAX_USD' apps/api/src apps/api/test --glob '*.ts'
printf '%s\n' '--- compact caller list ---'
rg -l 'upsertWalletSetting' apps/api/src apps/api/test --glob '*.ts' --glob '*.tsx' |
while read -r f; do
printf '\n--- %s ---\n' "$f"
rg -n -C 5 'upsertWalletSetting' "$f"
doneRepository: akash-network/console
Length of output: 26665
Enforce wallet-setting minimums before persistence.
The current production callers use WalletSettingsInputSchema, but upsertWalletSetting accepts direct WalletSettingInput calls. Add minimum validation for provided values in #validate, outside the autoReloadEnabled branch, before updateById or create.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts` at
line 61, Update the private `#validate` method in upsertWalletSetting to enforce
the wallet-setting minimums for every provided value, independently of the
autoReloadEnabled branch, before either updateById or create persists settings.
Reuse the existing minimum-validation rules or symbols, and preserve the current
autoReloadEnabled-specific validation.
Source: Path instructions
There was a problem hiding this comment.
Beyond the inline stale-comment nit, I checked the two job-scheduling concerns raised for wallet-settings.service.ts and didn't find them to hold up: the immediate-check call on threshold/amount change omits withCleanup, but the WalletBalanceReloadCheck queue uses policy singleton, which blocks concurrent active execution rather than additional created/scheduled jobs, so it doesn't collide with the pending safety-net job. And the immediate-check dispatch is intentionally flag-agnostic — the actual charge decision is gated by auto_reload_fixed_threshold inside the handler when the job runs, not at schedule time.
Extended reasoning...
I read wallet-reload-job.service.ts and job-queue.service.ts to check the singleton-collision claim: the queue is created with policy: handler.policy ("singleton"), which per pg-boss semantics restricts concurrent active execution per singleton key, not the presence of an already-scheduled/created job, so scheduleForWalletSetting(next) without withCleanup on a value change should enqueue successfully alongside a pending daily safety-net job rather than returning null and throwing. For the flag-gating concern, WalletBalanceReloadCheckHandler#tryToReload branches on FeatureFlags.AUTO_RELOAD_FIXED_THRESHOLD when the job actually runs, so scheduling the immediate check independent of the flag is correct — the flag controls the charge algorithm, not whether a check is worth running. This is informational only, based on examining these two candidates; it isn't a full review of the PR's scheduling logic.
|
|
||
| /** Upper bounds keep oversized input from overflowing the numeric(20, 2) columns (a 500) or being charged verbatim to the card. */ | ||
| const AUTO_RELOAD_THRESHOLD_MAX_USD = 10_000; | ||
| const AUTO_RELOAD_AMOUNT_MAX_USD = 10_000; |
There was a problem hiding this comment.
🟡 The JSDoc above AUTO_RELOAD_THRESHOLD_MAX_USD/AUTO_RELOAD_AMOUNT_MAX_USD says the $10,000 cap prevents overflowing a numeric(20, 2) column, but this same PR's integer-cents refactor (wallet-setting.schema.ts, migration 0035_cheerful_electro.sql) made autoReloadThreshold/autoReloadAmount integer columns, not numeric(20, 2). Integer comfortably holds far more than 1,000,000 cents ($10,000), so the overflow rationale is no longer accurate for these two fields — only the "charged verbatim to the card" half of the comment still holds. Suggest rewording to state the real justification (capping accidental huge charges).
Extended reasoning...
What's wrong: The comment at apps/api/src/billing/http-schemas/wallet.schema.ts:7 reads: Upper bounds keep oversized input from overflowing the numeric(20, 2) columns (a 500) or being charged verbatim to the card. This was accurate when it was written (commit b0def8a, added in response to a CodeRabbit review comment during this PR's review cycle, when auto_reload_threshold/auto_reload_amount were still numeric(20, 2) columns). But the PR's own later commit (9e38eee, "store auto top-up threshold and amount as integer cents") migrated wallet-setting.schema.ts to declare both columns as integer(...) (see migration 0035_cheerful_electro.sql: ADD COLUMN \"auto_reload_threshold\" integer), and the comment was never updated to match.\n\nWhy it's wrong now: With integer columns, the actual ceiling is PostgreSQL's int4 max of ~2.147 billion. The $10,000 cap in USD corresponds to 1,000,000 cents — nowhere close to that ceiling (roughly 2,147x smaller). So the "prevent overflowing the numeric(20, 2) columns" clause no longer describes a real risk for these two fields; there is nothing left to overflow at that value. The only clause that remains true is "being charged verbatim to the card," which is the actual reason a cap is still needed today: without it, a client-submitted value like $500,000 would pass validation and get charged directly to the user's payment method.\n\nWhy nothing else catches this: This is a pure documentation/comment-accuracy issue — TypeScript, tests, and runtime behavior are all unaffected, since the .max(10_000) validation itself is unchanged and still functions correctly regardless of what the comment says. Nothing in CI checks that a JSDoc comment's stated rationale matches the underlying schema.\n\nStep-by-step proof:\n1. Read apps/api/src/billing/http-schemas/wallet.schema.ts:6-9: the JSDoc justifies AUTO_RELOAD_THRESHOLD_MAX_USD/AUTO_RELOAD_AMOUNT_MAX_USD (both 10_000) by citing overflow of a numeric(20, 2) column.\n2. Read apps/api/src/billing/model-schemas/wallet-setting/wallet-setting.schema.ts: autoReloadThreshold and autoReloadAmount are declared as integer(...), not numeric(20, 2).\n3. Read apps/api/drizzle/0035_cheerful_electro.sql: confirms the migration adds both columns as integer.\n4. Convert the cap to the column's actual unit: $10,000 × 100 = 1,000,000 cents.\n5. Compare against integer's max value (~2,147,483,647): 1,000,000 is far below it, so no overflow is possible at the current cap — the stated risk doesn't exist for these columns anymore.\n6. Conclude: the "numeric(20, 2) overflow" half of the comment is stale; only the "charged verbatim to the card" half is still the operative justification.\n\nImpact and fix: Purely cosmetic — no behavioral or test impact, since the .max() validation logic is correct and unchanged. The fix is a one-line comment update, e.g.: Upper bound keeps oversized input from being charged verbatim to the card. This keeps the JSDoc's stated "why" accurate, per the repo's own clean-code-over-comments guidance that a why-comment should state the correct why.
2 participants
Why
Auto reload currently predicts upcoming spend: it projects the cost of all auto-top-up deployments over the next 7 days, triggers when the balance is below 25% of that projection, and charges
max(projection − balance, $20). Both charge timing and amount shift as deployments change, and the settings UI can only say "approximately $X per week".CON-717 replaces this with a fixed, user-configurable rule: when the credit balance is at or below threshold X, charge the default payment method exactly amount Y.
This is the backend half of CON-717, split out from the original combined PR. The frontend is in #3535 (stacked on this branch — it consumes the wallet-settings types regenerated here). This PR is safe to merge on its own: everything is behind a flag that is off by default.
Part of CON-717
What
Everything is gated behind a new feature flag
auto_reload_fixed_threshold(off by default). With the flag off, the legacy predicted-spend algorithm is untouched.auto_reload_threshold/auto_reload_amountcolumns (NUMERIC NOT NULL, defaults $20 / $100). The additiveADD COLUMN … DEFAULT … NOT NULLmigration backfills existing enabled users — that is the backfill story, no script.>= 5, amount>= $20, both optional on write (DB defaults on create).WalletBalanceReloadCheckHandlerbranches on the flag: flag on chargesmax(autoReloadAmount, $20)whenbalance <= autoReloadThreshold(inclusive), fires even with no active deployments, and never projects spend; flag off keeps the legacy path.createPaymentIntentunchanged (confirm: true, job-scoped idempotency key,onAmountMismatch: "tolerate").Behavior change at flag GA (needs a release note): enabled users with a balance ≤ their threshold get charged the fixed amount on their next check (spend-event-driven or the daily safety net). This was product-approved.
Rollout note for ops: in background jobs there is no unleash
userId, so the flag acts as a global on/off there (fine as a kill-switch, not per-user rollout). Metric changes (projected_cost_usd,zero_costreason) are deferred to the flag-cleanup follow-up.Follow-ups (filed separately): CON-763 — flag cleanup (remove the legacy algorithm, weekly-cost chain +
GET /v1/weekly-cost, old UI branch, and legacy metrics); CON-764 — notification on auto-top-up payment failure.Testing
PUT /v1/wallet-settings400s on amount < 20 / threshold < 5.docs.specsnapshot regenerated.npm run lint -- --quietandnpx tsc --noEmitclean for the changed files.Summary by CodeRabbit