feat: make generated SaaS overage delivery recoverable - #1794
Conversation
π WalkthroughWalkthroughSaaS μμ± ν νλ¦Ώμ΄ μμ billable usage journal, provider μ¬μλ·볡ꡬ, μ€λ³΅ μ²λ¦¬, plan version μ¦κ±° λ° billing drift μ§λ¨μ μ§μν©λλ€. CLI λμ보λμ μμ± μ± κ²μ¦λ μ μνμ contract canaryλ₯Ό κ²μ¬ν©λλ€. ChangesSaaS metered overage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
π₯ Pre-merge checks | β 4 | β 1β Failed checks (1 warning)
β Passed checks (4 passed)
β¨ Finishing Touches π‘ 1π Generate docstrings π‘
π§ͺ 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 |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4699e357a2
βΉοΈ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
π Benchmark Resultsβ All benchmarks passed
Updated: 2026-08-09T18:55:48.974Z Β· Commit: 37c3a8d |
There was a problem hiding this comment.
Actionable comments posted: 13
π€ 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/cli/src/commands/generateUsageDashboard.ts`:
- Around line 1126-1141: Update the Billing delivery section in
generateUsageDashboard.ts to render billingDelivery.oldestPendingAgeMs,
retryCount, and terminalFailureCount alongside the existing delivery metrics. In
packages/cli/src/tests/generateUsageDashboard.spec.ts lines 138-139, add
assertions that the generated page includes representations of all three values.
In `@packages/create-croco-app/src/node-runtime.ts`:
- Around line 19-33: SaaS Node runtime κ³μ½μ΄ μμ§ μ΅μ λ²μ κ³Ό λμΌν 22.5 pinμ μ¬μ©νλλ‘
λκΈ°ννμμμ€. packages/create-croco-app/src/node-runtime.tsμ
writeGeneratedNodeRuntimeContract()κ° engine rangeμ λ³λμ Node pinμ ν¨κ» λ°μ .nvmrcμ
appendGeneratedNodeGuidance()μ μ λ¬νλλ‘ λ³κ²½νκ³ ,
packages/create-croco-app/src/generator.ts 460-465μ SaaS preset νΈμΆμμ 22.5λ₯Ό
μ λ¬νμμμ€. scripts/create-croco-app-generated-smoke.mts 2456-2470μ μ±κ³΅ κ²°κ³Ό
nodeRecovery κ²μ¦λ 22.5 pinμ κΈ°λνλλ‘ κ°±μ νμμμ€.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/demo/FileBillableUsageJournal.ts`:
- Around line 217-230: Update normalizeEvent to canonicalize the top-level
BillableUsageEvent property order before stableEvent calls JSON.stringify, while
preserving the existing dimensions sorting. Ensure semantically identical events
with different insertion orders produce the same stableEvent value and remain
idempotent.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/demo/SqliteFixtureState.ts`:
- Around line 11-52: Rename the utility module containing
readSqliteFixtureState, updateSqliteFixtureState, and resetSqliteFixtureState to
sqliteFixtureState.ts, then update the corresponding import paths in
FileBillableUsageJournal.ts and FileUsageBillingGateway.ts. Preserve all
exported functions and behavior.
- Around line 70-81: Update replaceState so invalid target or source values
throw an appropriate `@croco/problems-core` Problem subclass instead of TypeError,
using the projectβs established Problem construction and diagnostic-code
conventions while preserving the existing validation behavior.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/demo/usage-recover.ts`:
- Around line 3-5: Add explicit rejection handling to the
recoverPendingBillableUsage entry point: write stable diagnostic information for
the failure to stderr and set a nonzero process exit code, while preserving the
existing stdout output for successful results.
In `@packages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.ts`:
- Around line 124-128: Update BILLABLE_USAGE_STATE_DIR so billable usage SQLite
state is isolated per process, either by honoring a process-specific
CROCO_DEMO_USAGE_STATE_DIR in the test setup or by including process.pid in the
default path. Keep the journal and provider paths derived from this directory,
and preserve the inherited environment so child recovery processes reuse the
same isolated state.
- Around line 1113-1121: The generated SaaS template must throw only Problem
subclasses with stable diagnostic codes instead of generic errors. In
packages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.ts#L1063,
1113-1121, 1189, 1197, and 1219, define or reuse dedicated Problem subclasses
for each failure and replace every throw new Error(...). In
packages/create-croco-app/templates/saas/apps/api-server/src/demo/SqliteFixtureState.ts#L70-L81,
update replaceState to throw a subclass from the same Problem hierarchy instead
of TypeError, preserving the existing failure conditions and messages where
applicable.
- Around line 1173-1192: Update recoverPendingBillableUsage so it does not
require OVERAGE_API_USAGE_EVENT_ID when no backlog exists. After delivery, look
up the entry optionally and validate that its state is "accepted" only when the
entry is present; preserve successful completion when the entry is absent.
- Around line 1194-1214: Update runUsageRecoveryProcess to give execFileAsync a
finite timeout, preventing the child process from hanging indefinitely. Parse
only the final non-empty stdout line, then validate the parsed object with the
existing zod dependency against the expected accepted, retryableFailed, and
terminalFailed numeric fields before returning it.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/tests/FileBillableUsageJournal.spec.ts`:
- Around line 74-91: Move the βpreserves concurrent provider ingestion and
duplicate acknowledgementβ test out of the FileBillableUsageJournal describe
block into a new FileUsageBillingGateway.spec.ts test file, preserving its
existing assertions and setup so it follows the required class-based test naming
convention.
- Around line 16-30: Rename the test around FileBillableUsageJournal append
behavior to reflect that it only verifies sequential appends, since synchronous
DatabaseSync operations inside Promise.all cannot interleave. Apply the same
scope-accurate naming to the analogous tests near the other referenced cases,
without claiming process-level SQLite locking or true concurrency.
- Line 15: FileBillableUsageJournal ν
μ€νΈμ EVENT_CONFLICT, INVALID_LEASE,
STALE_CLAIM κ° MeteringTransitionProblemμ μ€ν¨ κ²½λ‘λ₯Ό μΆκ°νμμμ€. λΉλκΈ° νΈμΆμ rejects.toThrowλ₯Ό
μ¬μ©ν΄ μ€λ₯λ₯Ό κ²μ¦νκ³ , κΈ°μ‘΄ μ±κ³΅ κ²½λ‘ ν
μ€νΈλ μ μ§νμμμ€.
πͺ 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4d05488f-f6b8-4c79-95fd-cc2aebc043cc
π Files selected for processing (24)
.changeset/prove-saas-metered-overage.mdpackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/create-croco-app/src/generator.tspackages/create-croco-app/src/node-runtime.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/monetization.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/FileBillableUsageJournal.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/FileUsageBillingGateway.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/SqliteFixtureState.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/saasSmokeContract.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/scenario.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/usage-recover.tspackages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/FileBillableUsageJournal.spec.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/SaasDemo.spec.tspackages/create-croco-app/templates/saas/apps/api-server/vitest.config.tspackages/create-croco-app/templates/saas/package.json.hbsscripts/create-croco-app-generated-smoke.mtsscripts/tests/create-croco-app-generated-smoke.spec.ts
8cbc97c to
fb4c475
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
π€ 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/cli/src/tests/generateUsageDashboard.spec.ts`:
- Around line 517-521: Update the generateUsageDashboard test fixture around
usageBillingReadModel.getSnapshot to accept and record the meterIds argument,
then assert that only required billing meters are passed. Add a meter without
billing classification to the meterRegistry fixture and verify it is excluded,
replacing the current string-based filter(isRequiredBillingMeter) check with an
assertion on the captured arguments.
In `@packages/create-croco-app/src/generator.ts`:
- Around line 461-469: Use the shared isSaasPreset type guard from options.ts
instead of duplicating the SaaS preset condition. In
packages/create-croco-app/src/generator.ts lines 461-469, evaluate isSaasPreset
once in finalize and reuse the result for both arguments to
writeGeneratedNodeRuntimeContract; in
packages/create-croco-app/src/cli-result.ts lines 53-61, import and use
isSaasPreset instead of computing a local condition in createSuccessResult.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/demo/sqliteFixtureState.ts`:
- Around line 41-46: Update the catch block in the transaction flow around
transactionStarted so a failed database.exec("ROLLBACK") is caught and
suppressed, then always rethrow the original caught error. Keep the existing
conditional rollback behavior and database.close() cleanup unchanged.
In `@packages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.ts`:
- Around line 132-141: Derive RecoveryDeliveryResult from recoveryDeliverySchema
with z.infer instead of maintaining a duplicate object type, and update the
delivery result handling around the existing βas RecoveryDeliveryResultβ cast to
rely on the inferred type without casting.
- Around line 285-297: Change getCurrentPlanVersion from an instance arrow
property to a prototype class method, matching the declaration style of
getCurrentPlanId while preserving its existing async logic and return behavior.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/tests/FileUsageBillingGateway.spec.ts`:
- Around line 13-32: νμΌ μ¬μ©λ κ²μ΄νΈμ¨μ΄ ν
μ€νΈμ provider μ₯μ κ²½λ‘λ₯Ό μΆκ°νμμμ€.
`FileUsageBillingGateway` μΈμ€ν΄μ€μμ `setAvailable(false)`λ₯Ό νΈμΆν λ€ `ingest`κ°
`PolarRetryableUpstreamProblem`μ throwνλμ§ `rejects.toThrow`λ‘ κ²μ¦νκ³ , κΈ°μ‘΄ μ μ μ²λ¦¬ ν
μ€νΈλ
μ μ§νμμμ€.
πͺ 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c2281570-ded6-4497-a5c0-7f9b0bf42f8b
β Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
π Files selected for processing (23)
.changeset/prove-saas-metered-overage.mddocs/problem-code-registry.jsonpackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/create-croco-app/src/cli-result.tspackages/create-croco-app/src/generator.tspackages/create-croco-app/src/node-runtime.tspackages/create-croco-app/src/tests/cli-result.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/demo/FileBillableUsageJournal.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/FileUsageBillingGateway.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/sqliteFixtureState.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/usage-recover.tspackages/create-croco-app/templates/saas/apps/api-server/src/problems.tspackages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/FileBillableUsageJournal.spec.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/FileUsageBillingGateway.spec.tspackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdscripts/create-croco-app-generated-smoke.mtstsconfig/contract-strict.baseline.json
π€ Files with no reviewable changes (1)
- packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
aadf667 to
20c3f5a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
π€ 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/create-croco-app/src/options.ts`:
- Around line 425-427: Update the public type predicate isSaasPreset so its
SaasPreset return type is externally resolvable: either export SaasPreset or
stop referencing it in the exported predicate signature by defining the
equivalent predicate type inline. Preserve the existing "saas" and "ai-saas"
narrowing behavior.
In `@packages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.ts`:
- Around line 988-994: Update the metering object in the saasDemo flow so
metering.recordedValue stores the local recorded usage total rather than
billableUsage.providerAcceptedUsage; reuse the existing currentUsage value if it
is that local aggregate, while leaving
usageBillingReadModel.providerAcceptedUsage as the provider-accepted evidence.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/tests/ExecutableAssurance.spec.ts`:
- Around line 20-23: Update the temporary directory construction in
ExecutableAssurance.spec.ts to use node:os tmpdir() instead of
process.env.TMPDIR with a /tmp fallback, matching the approach used by
FileUsageBillingGateway.spec.ts and preserving the existing
croco-executable-assurance path suffix.
πͺ 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e4c6d770-d2e9-455c-b7a6-042458c7f732
β Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
π Files selected for processing (12)
docs/problem-code-registry.jsonpackages/cli/src/tests/generateUsageDashboard.spec.tspackages/create-croco-app/src/cli-result.tspackages/create-croco-app/src/generator.tspackages/create-croco-app/src/options.tspackages/create-croco-app/templates/saas/apps/api-server/src/demo/sqliteFixtureState.tspackages/create-croco-app/templates/saas/apps/api-server/src/saasDemo.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/ExecutableAssurance.spec.tspackages/create-croco-app/templates/saas/apps/api-server/src/tests/FileUsageBillingGateway.spec.tspackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdscripts/create-croco-app-generated-smoke.mtstsconfig/contract-strict.baseline.json
20c3f5a to
522b154
Compare
522b154 to
279d1a6
Compare
PR μ΄ν΄νμΈλ λ²μ λ΄μμ PRμ ꡬ체μ μΈ λ³κ²½ μλλ₯Ό μμ½ν μ μμ΅λλ€. μμ€ν μν₯νμΈλ λ²μ λ΄μμ μν₯ λ°λ μμ€ν μμμ νμ ν μ μμ΅λλ€. Merge νλ¨β λ¨Έμ§ κ°λ₯ β λ¨Έμ§λ₯Ό λ§μ 리μ€ν¬ μμ
Must-fix
Recommended
ν΅μ¬ 리μ€ν¬νμΈλ λ²μ λ΄μμ λ¨Έμ§λ₯Ό λ§κ±°λ μ°μ μ‘°μΉκ° νμν ν΅μ¬ 리μ€ν¬λ λ°κ²¬λμ§ μμμ΅λλ€. ν λ¨κ³ κ°ν μ μνμΈλ λ²μ λ΄μμ μ°μ μ μν ν λ¨κ³ κ°ν λ°©ν₯μ΄ μμ΅λλ€. λ€μ μ€ν λ©λͺ¨νμΈλ λ²μ λ΄μμ λ€μ μ€ν μ ꡬ체μ μΌλ‘ μ 리ν μ μμ΅λλ€. μ 체 리뷰: ReviewLift λμ보λμ 리뷰 μμΈμμ νμΈνμΈμ. Review powered by ReviewLift |
Outcome
Generated SaaS applications now prove a credential-free fixed-subscription-plus-metered-overage path across local commit, provider outage, process-boundary recovery, duplicate acknowledgement, and final convergence. Stable JSON and Markdown evidence includes event IDs, delivery outcomes, backlog diagnostics, the recovery command, provider-accepted usage, and the shared
team@v1subscription and entitlement reference.The generated usage dashboard exposes only the required billing-delivery state: local and provider usage, drift, pending count and age, retry and terminal counts, and the recovery command. Monetization contract canaries reject unbound required meters with
CROCO_BILLING_METER_UNBOUNDand checkout-only provider profiles withCROCO_BILLING_PROVIDER_CAPABILITY_MISSING.Fixes #1528
Reviewer context
BEGIN IMMEDIATEtransactions so committed usage survives process exit without a hand-managed stale-lock path.>=22.5, the minimum release that providesnode:sqlite; other presets retain the existing Node contract.pnpm --dir apps/api-server demo:usage-recovercommand in a separate Node process.duplicatewithout increasing accepted usage.create-croco-apppreset behavior, and published recovery problem contracts.Verification
goal-saas-apiandai-saas-golden-pathgenerated-app paths on Node 22.23.1 β passed@croco/cliβ 30 files, 297 tests passed; lint, typecheck, and build passedcreate-croco-appβ 15 files, 144 tests passed; lint, typecheck, and build passedpnpm saas-billing-golden-path:smokeβ 10 passed after dependency-aware buildgit diff --check origin/trunk...HEADβ passedpnpm problem-registry:checkβ 606 codes from 606 discoveries passedReview gates