fix(deployment): reduce auto top-up escrow deposit sizing from 48h to 24h - #3562
fix(deployment): reduce auto top-up escrow deposit sizing from 48h to 24h#3562baktun14 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesAuto top-up configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes 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/deployment/config/env.config.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a single-line config default change (48 → 24), well-justified in the PR description and still env-overridable for instant rollback.
Extended reasoning...
Overview
The entire diff is a one-line change to apps/api/src/deployment/config/env.config.ts, lowering the zod default for AUTO_TOP_UP_AMOUNT_IN_H from 48 to 24. This value flows into the auto top-up job's escrow deposit sizing, the initial lease-start funding, and the estimatedTopUpAmount displayed in the UI — all downstream consumers just read the config value, no logic changes.
Security risks
None. No auth, crypto, or permission-sensitive code is touched. This only changes a numeric default that governs how much escrow is deposited per top-up cycle.
Level of scrutiny
Low. This is a tuning/config change with a clear, well-reasoned rationale in the PR description (reduces lump-sum draws and shortens the insufficient-balance window per CON-512) and an explicit rollback path via env override. I checked draining-deployment.service.spec.ts, which hardcodes AUTO_TOP_UP_AMOUNT_IN_H: 48 in its test env, so existing tests are unaffected by the default change.
Other factors
Grepped for other usages of AUTO_TOP_UP_AMOUNT_IN_H and confirmed there are only two consumers (the service and its spec), both already accounted for. No other file in the repo hardcodes the old 48h default.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3562 +/- ##
==========================================
- Coverage 75.27% 74.42% -0.86%
==========================================
Files 1163 1073 -90
Lines 30165 27816 -2349
Branches 7504 7044 -460
==========================================
- Hits 22708 20702 -2006
+ Misses 6585 6273 -312
+ Partials 872 841 -31
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
LGTM — trivial config default change with a clearly reasoned rationale and instant env-var rollback.
What was reviewed: confirmed the only change is the zod default (48→24) for AUTO_TOP_UP_AMOUNT_IN_H; checked its three consumers (hourly top-up sizing, lease-start funding, UI estimate) all read from config rather than hardcoding the old value; verified the existing unit test pins its own value (48) via mock config, so it isn't silently invalidated by the default change.
Extended reasoning...
Overview
Single-line change in apps/api/src/deployment/config/env.config.ts: the zod default for AUTO_TOP_UP_AMOUNT_IN_H drops from 48 to 24. This value drives escrow top-up sizing (hourly job, lease-start funding, and the UI's estimated top-up display), all of which read it from the shared config rather than hardcoding it, so no other code needed to change.
Security risks
None. This is a numeric tuning parameter for escrow deposit sizing, not an auth/permissions/crypto path. Worst case of a wrong value is over/under-funding escrow, not a security exposure.
Level of scrutiny
Low. This is a pure config/default value change with no control-flow modification, still overridable via env var for instant rollback, and the PR description gives a well-reasoned quantitative justification (halves deposit size and average locked escrow without shortening the safety buffer, since that's governed by the separate look-ahead window). CI (lint/tests/coverage) is green and the one existing spec that cares about this constant pins its own value explicitly, so it's unaffected by the default change.
Other factors
No CODEOWNER-sensitive paths touched, no outstanding review comments to address, and CodeRabbit found nothing actionable. This is exactly the kind of small, well-justified, mechanical change suited for approval without further human review.
1 participant
Why
The hourly auto top-up adds a flat
AUTO_TOP_UP_AMOUNT_IN_H(48h) worth of a deployment's cost on top of whatever it already holds, and it triggers while the deployment can still hold up to 24h of runway. Expensive deployments therefore hold up to ~72h of cost, and a single funding event moves 48h of burn at once: a $5/h deployment pulls $240 per deposit and holds up to ~$360 in escrow. That lump drains the Available balance in one shot, chains consecutive auto-reload card charges for anything burning more than ~$2/h against the $100 default reload amount, and widens the insufficient-balance window for new deployment creation (CON-512).Halving the flat amount to 24h halves each draw and the average locked escrow without shortening the buffer against a broken top-up job: time to first customer impact is set by the separate 24h look-ahead trigger (
AUTO_TOP_UP_LOOK_AHEAD_WINDOW_IN_H), not by the deposit size. First closures after a dead cron still start ~24h in; only the tail of the fleet drains sooner (~48h instead of ~72h).Related to CON-791: to-target sizing remains the proper fix. A flat 24h add at the 24h trigger is roughly equivalent to CON-791's 48h target in steady state, so this value should be revisited when that lands.
What
AUTO_TOP_UP_AMOUNT_IN_Hzod default from 48 to 24. This sizes the hourly escrow top-up, the initial funding at lease start, and theestimatedTopUpAmountvalue shown in the UI.AUTO_TOP_UP_AMOUNT_IN_H=48is an instant rollback with no deploy.Summary by CodeRabbit