Add script to expire non-expiring free credits#1269
Conversation
introduce configurable concurrency for expire-free-credits via p-limit parse --concurrency, defaulting to 50 and validate values increase default batch size to 10000 for throughput create timestamped output and error log files under output/ log created file paths and active concurrency for visibility
…edits exclude categories from expiration to avoid expiring credits exclude list: orb_migration_accounting_adjustment, credits_expired custom, usage_issue, feedback
…category first Instead of scanning all users and filtering, the script now takes a mandatory --category=<name> arg and queries credit_transactions by category directly, then groups by user. Much faster for targeted runs.
…ry-based filtering
…n pairs - Replace --category param with embedded (category, description) pairs copied from the reviewed spreadsheet - Empty description matches NULL or empty, specific description matches exactly — handles same category with different descriptions - Expiry date is now 30 days from runtime instead of hardcoded - Add per-category breakdown in summary output - Improve progress logging clarity
ignore runtime artifacts produced by the Kilo agent in prettier
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Fix these issues in Kilo Cloud Other Observations (not in diff)None. Files Reviewed (2 files)
Reviewed by gpt-5.4-20260305 · 294,119 tokens |
markijbema
left a comment
There was a problem hiding this comment.
I mean i cant verify correctness just by reviewing. I think I'd write some tests or run on a copy of the database to verify first
Covers: fully/partially/unspent users, any-description matching, non-free exclusion, org-scoped exclusion, already-expiring exclusion, wrong description, mixed credits, multiple matching credits, zero-amount, existing next_credit_expiration_at LEAST, and multi-block projected expiration correctness.
… comment the code comment now uses EXPIRY_DATE instead of a fixed date to clarify runtime behavior and enable easier configuration of the expiry date used by the script
…cases Verifies that original_baseline_microdollars_used correctly determines whether free credits are covered by prior usage: free credits granted after spending are not covered and expire fully, while free credits granted before spending are covered and nothing expires.
…its across batches Uses a subquery to select the next N distinct user IDs, then fetches all matching credits for those users in a single query. Prevents the previous row-level pagination from skipping credits when a user's rows span a batch boundary. Test runs with --batch-size=1 to exercise this.
…Orb double-deductions When Orb clawed back spent free credits (reducing total_microdollars_acquired), the expiration simulation would still try to expire the full credit amount, pushing ~1,610 users into negative balance. Now boosts expiration baselines on newly-tagged credits so total expiration never exceeds the current balance.
- Add src/scripts/ to testPathIgnorePatterns (integration tests need POSTGRES_SCRIPT_URL which isn't available in CI) - Replace non-null assertions with null checks / fallbacks
…ve instead of boosting baselines Baseline boosting was wrong — increasing a credit's baseline shifts its claim window right, which *increases* expiration, not decreases it. With microdollars_used=0 (Orb users), no baseline prevents full expiration. New approach: simulate all expirations, compute headroom (balance minus existing expirations), then only set expiry on credits that fit within headroom. Credits that would cause over-expiration are skipped entirely.
…fter-script Replace the misleading single projected_balance_microdollars with two fields: - projected_no_script_microdollars: balance after only pre-existing expirations - projected_after_script_microdollars: balance after script-set expirations too
The expire script now writes a mutations.jsonl file recording every DB change (old/new values for each credit transaction and user row). New revert script reads the mutations file and undoes the changes: - Resets expiry_date and expiration_baseline to null on credit transactions - Recomputes next_credit_expiration_at from remaining expiring credits Usage: pnpm script src/scripts/d2026-03-18_expire-free-credits-revert.ts <mutations-file> [--execute]
Runs expire script → takes snapshot → runs revert script → verifies all credits and users are restored to their original state.
…d values - Move mutation logging after the DB transaction commits so rolled-back transactions don't leave phantom entries in the mutations file - Revert script now restores the exact old values from the mutations log instead of blindly writing nulls
Both the expire and revert scripts now show the target database hostname and prompt for confirmation before proceeding. Bypass with --yes/-y. Tests assert the DB URL is local postgres and pass --yes automatically.
There was a problem hiding this comment.
There are two issues when comparing with the audit sheet.
orb_free_credits | (any) sweeps up entries the audit marked SHOULD_EXPIRE=FALSE: "Email $50 non-expire (BDO)" at $2,250 across 45 credits, and "100 email campaign (Justin)" at $100. A few unreviewed entries come along too. The FALSE ones were explicitly opted out during the review, so running --execute as-is would override those decisions.
The second is the expiry window for custom | Part-time UX hire, providing tokens to use product and be productive: the audit has EXPIRE_IN_DAYS=180, but the script applies 30 days to everything. Just one credit at $100, so not a big deal financially, but it goes against the reviewed call.
| const mutationsFile = findLatestMutationsFile(); | ||
| console.log(`Running revert script with mutations file: ${mutationsFile}\n`); | ||
| const revertOutput = execSync( | ||
| `pnpm script src/scripts/d2026-03-18_expire-free-credits-revert.ts ${mutationsFile} --execute --yes`, |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the fix is to avoid constructing a shell command string that embeds unescaped, dynamic values (like filenames) and then running it through a shell. Instead, invoke the command with an API that accepts the executable and its arguments separately (for example, execFileSync or spawnSync from node:child_process), passing mutationsFile as a distinct argument so the shell does not interpret it.
Concretely here, we should:
- Replace the use of
execSyncwithexecFileSyncfor both command invocations:- The “expire-free-credits” script call on lines 214–217.
- The “revert” script call on lines 232–235, which is where the tainted
mutationsFileis interpolated.
- Pass
"pnpm"as the command,"script"as the first argument, followed by the script path and other flags, and in the second call includemutationsFileas its own argument in the args array. - Import
execFileSyncfromnode:child_processinstead ofexecSync. - Preserve the existing options (
cwd,encoding,env,timeout) by passing the same options object toexecFileSync.
All of these changes must be made within src/scripts/d2026-03-18_expire-free-credits-revert.test.ts:
- Update the import on line 15 from
execSynctoexecFileSync. - Change the
expireOutputassignment on lines 214–217 to callexecFileSync('pnpm', [...args...], options). - Change the
revertOutputassignment on lines 232–235 similarly, ensuringmutationsFileis in the args array and not interpolated into a single command string.
No additional helper methods are required; only the import and function calls need updating.
| @@ -12,7 +12,7 @@ | ||
|
|
||
| import '../lib/load-env'; | ||
|
|
||
| import { execSync } from 'node:child_process'; | ||
| import { execFileSync } from 'node:child_process'; | ||
| import { readdirSync, writeFileSync, unlinkSync, mkdtempSync } from 'node:fs'; | ||
| import { tmpdir } from 'node:os'; | ||
| import path from 'node:path'; | ||
| @@ -211,8 +211,16 @@ | ||
|
|
||
| // 2. Run the expire script | ||
| console.log('Running expire-free-credits script with --execute...\n'); | ||
| const expireOutput = execSync( | ||
| `pnpm script src/scripts/d2026-03-18_expire-free-credits.ts --input=${testCsvPath} --execute --yes --batch-size=1`, | ||
| const expireOutput = execFileSync( | ||
| 'pnpm', | ||
| [ | ||
| 'script', | ||
| 'src/scripts/d2026-03-18_expire-free-credits.ts', | ||
| `--input=${testCsvPath}`, | ||
| '--execute', | ||
| '--yes', | ||
| '--batch-size=1', | ||
| ], | ||
| { cwd: process.cwd(), encoding: 'utf-8', env: { ...process.env }, timeout: 120_000 } | ||
| ); | ||
| console.log(expireOutput); | ||
| @@ -229,8 +237,15 @@ | ||
| // 4. Run the revert script | ||
| const mutationsFile = findLatestMutationsFile(); | ||
| console.log(`Running revert script with mutations file: ${mutationsFile}\n`); | ||
| const revertOutput = execSync( | ||
| `pnpm script src/scripts/d2026-03-18_expire-free-credits-revert.ts ${mutationsFile} --execute --yes`, | ||
| const revertOutput = execFileSync( | ||
| 'pnpm', | ||
| [ | ||
| 'script', | ||
| 'src/scripts/d2026-03-18_expire-free-credits-revert.ts', | ||
| mutationsFile, | ||
| '--execute', | ||
| '--yes', | ||
| ], | ||
| { cwd: process.cwd(), encoding: 'utf-8', env: { ...process.env }, timeout: 120_000 } | ||
| ); | ||
| console.log(revertOutput); |
…matching logic - Remove hardcoded template literal credit category/description data and global EXPIRY_DATE constants - Add --input=<path> required CLI argument for CSV file - Add parseCsv() that validates columns, parses rows, deduplicates pairs, and builds a Map<PairKey, RowLookup> with pre-computed per-row expiry dates - Add resolveCredit() with specific-match-then-catch-all fallback - Update processUser() to accept lookup map, resolve per-credit expiry dates, update each credit individually in the DB transaction, and use earliest expiry for next_credit_expiration_at - Replace OR condition query filter with simpler inArray on categoriesToQuery, with app-side filtering via resolveCredit after fetch - Remove unused `or` import from drizzle-orm
When headroom is limited (Orb clawback cases), the order in which credits are consumed matters. Sort by resolved expiry date ascending so earlier-expiring credits are preferred over later-expiring ones.
Summary
total_microdollars_acquired) and skips setting expiry on credits that would push the balance negative--executeto write changesprojected_no_script/projected_after_scriptbalances for clear before/after visibilityKey details
Test plan