Repository navigation
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| origin: normalizedOrigin(metadata.identity.billingOrigin), | ||
| ...(metadata.workspace?.sandboxProvider === 'cloudflare-containers' && | ||
| subject.type === 'org' | ||
| ? computeMemberMetadata(actor, metadata.identity.userId) |
There was a problem hiding this comment.
CRITICAL: Acting-member billingUserId never reaches the meter
This branch puts the acting member into metadata.billingUserId, and assertSandboxBillingAllocation now permits that key. But every meter generation is created by createBillingGeneration (services/cloud-agent-next/src/metered-billing-lifecycle.ts:921-928), which rebuilds metadata from an allow-list (container_class, durable_object_id, vcpu, memory_mib, disk_mb, origin) and never copies input.metadata.billingUserId; usageContextFromBillingContext then sends that metadata in recordStart. For subject.type === 'org' with a bot actor the meter's applyStartWithDb (services/container-usage-meter/src/postgres.ts:506) throws UsageMutationConflictError, which billingAdmissionFailureFromError maps to meter_unavailable. Org bot sessions therefore retry admission for the full one-minute window and fail billing_unavailable, and pre-upgrade org bot intervals settle with remaining = 0. The new test only asserts the buildSandboxBillingInput result, so the stored context is not covered. Copy billingUserId into the stored metadata (or stop rebuilding metadata there).
| ...(pin.billing === null ? {} : { billing: pin.billing }), | ||
| }; | ||
| if (pin.provider === 'cloudflare-containers') { | ||
| billingAdmissionDeadlineAt = await nativeBillingAdmissionDeadlineAt( |
There was a problem hiding this comment.
WARNING: Retry create can reopen admission after the alarm already failed the sandbox
The new alarm branch above (sandbox-do.ts:1892-1905) runs failCreationRoutesNow(... 'billing_unavailable') as soon as the stored deadline passes, including while runCreate is still in its runtime-artifact/launch-env preflight (the alarm only defers on createInFlight after that block). runCreate validates the allocation only at entry (line 4240). If the alarm fires during that window it clears the key and fails the routes; this retry path then finds no preparing route, writes a fresh now + billingAdmissionMs deadline against the stale allocationId, calls armAlarm(state) and proceeds with provider.create(...), performing meter admission and launching a container for a request already reported as billing_unavailable (the post-create stopRef cleans it up, but the meter still sees a phantom start/stop interval). Re-check state.kind/allocationId and that the attempt is still preparing after the preflight, before writing the deadline or creating.
| @@ -0,0 +1 @@ | |||
| CREATE INDEX "IDX_compute_usage_charge_created" ON "compute_usage_charge" USING btree ("created_at"); No newline at end of file | |||
There was a problem hiding this comment.
SUGGESTION: Partitioned index build blocks settlement inserts
compute_usage_charge is a monthly-partitioned, write-hot ledger (every paid segment inserts a row), so this parent CREATE INDEX locks the parent and each existing partition and blocks settlement inserts until commit. The RUNBOOK documents scheduling a low-traffic window, but the migration has no lock_timeout and no guarded path, so a large partition can stall the meter's settlement path for the whole build. Consider the runbook's per-partition CONCURRENTLY + ATTACH build, or at least SET LOCAL lock_timeout so a large partition fails fast.
| const request = { | ||
| allocationRef: record.allocationRef, | ||
| billingFailure: billingFailureFromAdmission(context.subject, { | ||
| code: budget?.code === 'BILLING_UNAVAILABLE' ? 'meter_unavailable' : 'insufficient_credits', |
There was a problem hiding this comment.
SUGGESTION: A block without code is reported as a credit shortage
enforceBudgetStop writes block.code only when it creates a new block (metered-billing-lifecycle.ts:608-616), so a block persisted by the previous release has no code. Replaying such a block through onContainerStarted/enforceBudgetStop reaches this mapping with budget.code === undefined and sends INSUFFICIENT_CREDITS to the customer even when the stop came from an unreadable meter verdict, which is exactly the case that must not claim a credit shortage. Treat an absent code as BILLING_UNAVAILABLE, or apply failureCode to an existing block too.
| if (billingMode === 'paid') { | ||
| const remaining = await balanceForSubject(tx, input.subject, true); | ||
| if (remaining <= MINIMUM_REMAINING_MICRODOLLARS) { | ||
| if (isNativeOrganizationUsage(input) && !organizationMemberId(input)) { |
There was a problem hiding this comment.
SUGGESTION: Unattributed native org start is reported as a transient meter outage
This throws UsageMutationConflictError, which billingAdmissionFailureFromError maps to meter_unavailable, so the client keeps retrying for the whole admission budget and then reports a retryable billing outage. An unattributed org actor is a permanent configuration problem (the RUNBOOK says such starts 'fail billing verification until their configuration is corrected'), so it should fail fast like the new non-retryable invalid_configuration SKU path instead of consuming create retries.
| `); | ||
| let rebuiltScopes = 0; | ||
| for (const { scope_key: scopeKey } of scopes.rows) { | ||
| await sweepHourlyBuckets(database, { lookbackHours: 720, now, scopeKey }); |
There was a problem hiding this comment.
SUGGESTION: Unbounded backfill loop has no batching, progress or resume
scopes.rows is fully materialized and the loop has no limit, batch size or cursor, so a large backfill is all-or-nothing and an interrupted run restarts every payer from zero, re-running the 720-hour sweep each time. Chunk the scope list (and log progress) so operators can resume instead of rerunning all scopes.
| asc(charge.rate_cents_per_unit) | ||
| ) | ||
| .limit(COMPUTE_CHARGE_PAGE_SIZE + 1) | ||
| .offset(input.page * COMPUTE_CHARGE_PAGE_SIZE); |
There was a problem hiding this comment.
SUGGESTION: Offset pagination re-aggregates the range and can drift between pages
Each page re-runs the grouped/sorted aggregation over up to a 366-day range only to discard page * 100 rows (page is allowed up to 1000). Because the ledger is append-only and the order starts with desc(day), a charge written between page requests shifts groups across the offset, so rows can be duplicated or skipped. Keyset pagination on the existing order columns (or a much lower page cap) avoids both.
Code Review SummaryStatus: 7 Issues Found | Recommendation: Address before merge Executive SummaryThe acting-member Assumptions: Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (106 files)
Reviewed by deepseek-flash-latest · Input: 178.7K · Output: 51.6K · Cached: 8.2M Review guidance: REVIEW.md from base branch |
Summary
Native Cloudflare Containers can lose the final usage interval during a controlled stop. Organization compute also needs member accounting, billing recovery, and customer charge visibility.
Stacked on #8274. This PR contains one implementation commit above that branch.
Verification
Prior local implementation checks recorded in this worktree:
These checks establish local behavior. Hosted restart behavior, production timing, payment-provider behavior, and Snowflake ingestion remain unverified.
Visual Changes
Compute pricing and billing recovery now appear in Cloud Agent. Compute charges appear in usage analytics.
Reviewer Notes
Automated results from the prior local verification report:
For PR publication,
pnpm format:checkandgit diff --check HEAD~1 HEADpassed again. Other results above come from the existing local verification report.Review settlement replay, attribution, allowance enforcement, and admission recovery. Member limits apply to native compute in this release; SDK, Vercel, and Gastown accounting retain their existing behavior.
Release work is separate: verify migration 0297's index lock window and the production script target. Follow
services/container-usage-meter/RUNBOOK.mdfor deployment order and immediate scoped backfill. Historical compute can leave an incomplete anomaly baseline during the deploy/backfill gap. External monitors and provider reconciliation require operations checks.