Skip to content

Commit b7394be

Browse files
authored
fix: release-review follow-ups for account removal (workflow-state rejections, stats-row creator, sweep pages) (#9164)
* fix(trigger): complete runs whose workflow was deleted or undeployed before they started Not-found, archived, undeployed, unattached and suspended rejections come from a successful read and never wait on an execution log (not-found and undeployed never write one), so they complete without the hasCompleted gate. Quota rejections (402/429) still complete only once their log is saved. * fix(projects): finish a retired Project's storage sweep only on the provider's last page Azure Blob, GCS and S3-compatible providers can return a short or empty page that is not the last, which the sweep read as done and left the rest of the prefix orphaned. The sweep now ends only on page.done, and follows a non-final page that removed nothing to the next one within the run. * fix(account-deletion): keep a public object a stored URL with a fragment still names A relative /api/files/serve/<key>#frag kept the fragment in its candidate key, so a live reference could be missed and the object purged. The guard now also reads the path without its fragment, on both the decoded and fallback paths. * fix(billing): withhold the free allowance from stats rows created outside signup A storage write or an ownership transfer that creates a missing user_stats row gave it the default free allowance without the deleted-identity check ensureUserStatsExists makes. When either insert creates the row, the same email-only check now runs on the caller's transaction, in a savepoint that fails open; an existing row costs no extra query. * docs(async-jobs): scope cancelByWorkspace to persisted queue jobs * refactor(trigger): classify preprocessing rejections by meaning, not status code Preprocessing tags not-found, archived, not-attached and not-deployed rejections with an internal workflowState (isWorkflowStateRejection); these write no execution log by design, so the job completes at once. Quota and account rejections (usage limit, spent reservation headroom, rate limit, suspended account) complete only once their log is saved; a suspended account goes back behind that gate. The tag is a separate field rather than a code because v2 execute, its streaming error and MCP serve copy error.code into their responses; a new code on a 404 would change those outputs. Headroom denials are matched through a new isReservationHeadroomDenial next to their descriptors instead of joining the deterministic-rejection set, which webhooks and polling read to stop retries and which deliberately leaves headroom out. * refactor(billing): create every fallback user_stats row through one creator createUserStatsIfMissing inserts the row on the caller's executor and, only when it created the row, runs the deleted-identity check and forfeits the allowance (a transaction on db, a savepoint inside a caller's transaction), failing open. ensureUserStatsExists, the storage write and the ownership transfer all use it; the two forfeit paths merge into one. One behavior delta: a stats row the ownership transfer creates for the new owner now starts at getFreeTierLimit() instead of the schema default, the same as every other creator (identical unless FREE_TIER_COST_LIMIT is set). The admin credits route keeps its own insert: it is not behavior-identical (short id, no usageLimitUpdatedAt, a plain insert that throws on a concurrent create rather than a no-op), and it rewrites the limit right after. * refactor(billing): drop the unused executor from the deleted-identity email check isDeletedAccountIdentity reads on db as it did before; only userHasDeletedAccountIdentity takes a caller's transaction, so its savepoint wrapper moves inline. Hoist the headroom descriptors to a module constant.
1 parent c5c1a1e commit b7394be

17 files changed

Lines changed: 491 additions & 117 deletions

‎apps/sim/background/async-preprocessing-correlation.test.ts‎

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -682,36 +682,79 @@ describe('async preprocessing correlation threading', () => {
682682
}
683683

684684
it.each([
685-
{ statusCode: 404, message: 'Workflow not found' },
686-
{ statusCode: 403, message: 'Workflow is not deployed' },
687-
{ statusCode: 402, message: 'Usage limit exceeded. Please upgrade your plan to continue.' },
688-
{ statusCode: 429, message: 'Rate limit exceeded. Please try again later.' },
689-
])('completes the job for a non-retryable $statusCode', async ({ statusCode, message }) => {
685+
{ statusCode: 404, message: 'Workflow not found', workflowState: 'not_found' },
686+
{
687+
statusCode: 403,
688+
message: 'Workflow is not deployed',
689+
code: 'WORKFLOW_NOT_DEPLOYED',
690+
workflowState: 'not_deployed',
691+
},
692+
{
693+
statusCode: 403,
694+
message:
695+
'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot execute.',
696+
workflowState: 'not_attached',
697+
},
698+
])('completes the job for a $statusCode $message that never started a log', async (error) => {
699+
loggingSessionMockFns.mockHasCompleted.mockReturnValueOnce(false)
700+
701+
await expect(runRejected(error)).resolves.toMatchObject({
702+
success: false,
703+
workflowId: 'workflow-1',
704+
executionId: 'execution-rejected',
705+
error: error.message,
706+
})
707+
})
708+
709+
const loggedRejections = [
710+
{
711+
statusCode: 402,
712+
message: 'Usage limit exceeded. Please upgrade your plan to continue.',
713+
code: 'USAGE_LIMIT_EXCEEDED',
714+
},
715+
{
716+
statusCode: 402,
717+
message: 'Usage headroom exhausted',
718+
code: 'EXECUTION_PAYER_HEADROOM_EXHAUSTED',
719+
retryable: false,
720+
},
721+
{
722+
statusCode: 429,
723+
message: 'Rate limit exceeded. Please try again later.',
724+
code: 'RATE_LIMIT_EXCEEDED',
725+
},
726+
{ statusCode: 403, message: 'Account suspended', code: 'ACCOUNT_SUSPENDED' },
727+
]
728+
729+
it.each(loggedRejections)('completes the job for a logged $code rejection', async (error) => {
690730
loggingSessionMockFns.mockHasCompleted.mockReturnValueOnce(true)
691-
await expect(runRejected({ statusCode, message })).resolves.toMatchObject({
731+
732+
await expect(runRejected(error)).resolves.toMatchObject({
692733
success: false,
693734
workflowId: 'workflow-1',
694735
executionId: 'execution-rejected',
695-
error: message,
736+
error: error.message,
696737
})
697738
expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalledWith(
698-
expect.objectContaining({ error: expect.objectContaining({ message }) })
739+
expect.objectContaining({ error: expect.objectContaining({ message: error.message }) })
699740
)
700741
})
701742

702-
it('fails the job when the rejection could not be logged', async () => {
703-
loggingSessionMockFns.mockHasCompleted.mockReturnValueOnce(false)
743+
it.each(loggedRejections)(
744+
'fails the job when a $code rejection could not be logged',
745+
async (error) => {
746+
loggingSessionMockFns.mockHasCompleted.mockReturnValueOnce(false)
704747

705-
await expect(
706-
runRejected({ statusCode: 403, message: 'Workflow is not deployed' })
707-
).rejects.toThrow('Workflow is not deployed')
708-
})
748+
await expect(runRejected(error)).rejects.toThrow(error.message)
749+
}
750+
)
709751

710752
it.each([
711753
{ statusCode: 429, message: 'Too many concurrent executions', retryable: true },
712754
{ statusCode: 402, message: 'Usage is temporarily unavailable', retryable: true },
713755
{ statusCode: 503, message: 'Usage admission is temporarily unavailable', retryable: true },
714756
{ statusCode: 500, message: 'Internal error while fetching workflow' },
757+
{ statusCode: 404, message: 'Workflow not found' },
715758
])('fails the job for $statusCode $message', async (error) => {
716759
await expect(runRejected(error)).rejects.toThrow(error.message)
717760
})

‎apps/sim/background/workflow-execution.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
assertBillingAttributionSnapshot,
1717
type BillingAttributionSnapshot,
1818
} from '@/lib/billing/core/attribution'
19+
import { getDeterministicAdmissionRejectionCode } from '@/lib/core/admission/rejection'
20+
import { isReservationHeadroomDenial } from '@/lib/core/admission/transient-failure'
1921
import { buildRecordedJobFailureResult } from '@/lib/core/async-jobs/recorded-failure'
2022
import type { AsyncExecutionCorrelation } from '@/lib/core/async-jobs/types'
2123
import type { ExecutionScope } from '@/lib/core/errors/execution-scope-deleted'
@@ -30,7 +32,12 @@ import {
3032
RESERVATION_TTL_BUFFER_MS,
3133
} from '@/lib/core/execution-limits'
3234
import type { RequestAttribution } from '@/lib/core/utils/request-attribution'
33-
import { type PreprocessExecutionError, preprocessExecution } from '@/lib/execution/preprocessing'
35+
import {
36+
isWorkflowStateRejection,
37+
type PreprocessExecutionError,
38+
preprocessExecution,
39+
RATE_LIMIT_EXCEEDED_CODE,
40+
} from '@/lib/execution/preprocessing'
3441
import { LoggingSession } from '@/lib/logs/execution/logging-session'
3542
import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans'
3643
import { cleanupExecutionBase64Cache } from '@/lib/uploads/utils/user-file-base64.server'
@@ -109,18 +116,23 @@ type LegacyWorkflowExecutionPayload = Omit<WorkflowExecutionPayload, 'principal'
109116
type WorkflowExecutionJobPayload = WorkflowExecutionPayload | LegacyWorkflowExecutionPayload
110117

111118
/**
112-
* Preprocessing rejections that are an outcome of the workflow's or account's
113-
* state — deleted or undeployed workflow, suspended account, exhausted usage
114-
* or rate quota — rather than a fault. A retryable rejection (concurrency
115-
* capacity, an unreadable usage ledger, reservation infrastructure) and every
116-
* 5xx still fail the job.
119+
* Whether a preprocessing rejection is an outcome of the workflow's or account's state rather
120+
* than a fault. A workflow-state rejection writes no execution log, so it completes at once. A
121+
* quota or account rejection (exhausted usage, spent headroom, rate limit, suspended account)
122+
* writes one, so it completes only once that log is saved; an unsaved one (an outage) still fails
123+
* the job and alerts. A retryable rejection and every other failure fail the job.
117124
*/
118-
const NON_FAULT_PREPROCESSING_STATUS_CODES: ReadonlySet<number> = new Set([402, 403, 404, 429])
119-
120-
function isNonFaultPreprocessingRejection(rejection: PreprocessExecutionError): boolean {
121-
return (
122-
NON_FAULT_PREPROCESSING_STATUS_CODES.has(rejection.statusCode) && rejection.retryable !== true
123-
)
125+
function isNonFaultPreprocessingRejection(
126+
rejection: PreprocessExecutionError,
127+
rejectionLogged: boolean
128+
): boolean {
129+
if (rejection.retryable === true) return false
130+
if (isWorkflowStateRejection(rejection)) return true
131+
const quotaRejection =
132+
getDeterministicAdmissionRejectionCode(rejection) !== undefined ||
133+
isReservationHeadroomDenial(rejection) ||
134+
rejection.code === RATE_LIMIT_EXCEEDED_CODE
135+
return quotaRejection && rejectionLogged
124136
}
125137

126138
/** Completes the job with a failure already in the execution log, keeping it on the run's span. */
@@ -378,11 +390,9 @@ export async function executeWorkflowJob(
378390
const scopeDeletedOnCompletion = loggingSession.getDeletedScope()
379391
if (scopeDeletedOnCompletion) return completeWithDeletedScope(scopeDeletedOnCompletion)
380392

381-
// Complete only when the rejection is durably logged; an unsaved one must still alert.
382393
if (
383394
preprocessingRejection &&
384-
isNonFaultPreprocessingRejection(preprocessingRejection) &&
385-
loggingSession.hasCompleted()
395+
isNonFaultPreprocessingRejection(preprocessingRejection, loggingSession.hasCompleted())
386396
) {
387397
return completeWithRecordedFailure(workflowId, executionId, error)
388398
}

‎apps/sim/lib/billing/core/usage.ts‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ export async function handleNewSignInAccount(
262262
): Promise<void> {
263263
if (!isUserCreatedInAuthRequest(userId, authRequest)) return
264264
if (!(await isDeletedAccountIdentity({ oauthSubjects: [{ providerId, accountId }] }))) return
265-
await forfeitFreeTierAllowance(userId)
265+
await forfeitFreeTierAllowance(db, userId)
266266
}
267267

268268
/**
@@ -283,29 +283,45 @@ export async function ensureUserStatsExists(
283283
userId: string,
284284
authRequest?: object | null
285285
): Promise<void> {
286-
const created = await db
286+
await createUserStatsIfMissing(db, userId, {
287+
includeSignIns: isUserCreatedInAuthRequest(userId, authRequest),
288+
})
289+
}
290+
291+
/**
292+
* Creates a user's `user_stats` row with the free allowance when none exists, on `executor`
293+
* (the caller's transaction, or `db`). Only a row this call creates gets the deleted-identity
294+
* check signup makes: the email always, and the linked sign-ins only when `includeSignIns` is
295+
* set. An existing row costs only the conflicting insert. A failed check leaves the allowance in
296+
* place and never fails the caller or aborts its transaction.
297+
*
298+
* @returns whether this call created the row
299+
*/
300+
export async function createUserStatsIfMissing(
301+
executor: DbOrTx,
302+
userId: string,
303+
{ includeSignIns = false }: { includeSignIns?: boolean } = {}
304+
): Promise<boolean> {
305+
const created = await executor
287306
.insert(userStats)
288307
.values({
289308
id: generateId(),
290-
userId: userId,
309+
userId,
291310
currentUsageLimit: getFreeTierLimit().toString(),
292311
usageLimitUpdatedAt: new Date(),
293312
})
294313
.onConflictDoNothing({ target: userStats.userId })
295314
.returning({ id: userStats.id })
296-
if (created.length === 0) return
315+
if (created.length === 0) return false
297316

298317
try {
299-
const includeSignIns = isUserCreatedInAuthRequest(userId, authRequest)
300-
if (await userHasDeletedAccountIdentity(userId, { includeSignIns })) {
301-
await forfeitFreeTierAllowance(userId)
318+
if (await userHasDeletedAccountIdentity(userId, { includeSignIns }, executor)) {
319+
await forfeitFreeTierAllowance(executor, userId)
302320
}
303321
} catch (error) {
304-
logger.error('Failed to withhold the free allowance from a fallback stats row', {
305-
userId,
306-
error,
307-
})
322+
logger.error('Failed to withhold the free allowance from a new stats row', { userId, error })
308323
}
324+
return true
309325
}
310326

311327
interface ResolvedUserUsageData {
@@ -690,12 +706,12 @@ export async function checkUsageStatus(userId: string): Promise<{
690706
/**
691707
* Withholds the free allowance from a new account found to reuse the sign-in
692708
* identity of a recently deleted one after its stats row was created. The flag
693-
* and the limit it implies commit together, so a failed re-derivation can never
694-
* leave the account flagged but still holding the free limit; repeating the
695-
* call is harmless.
709+
* and the limit it implies commit together (a transaction on `db`, a savepoint
710+
* inside a caller's transaction), so a failed re-derivation can never leave the
711+
* account flagged but still holding the free limit; repeating the call is harmless.
696712
*/
697-
async function forfeitFreeTierAllowance(userId: string): Promise<void> {
698-
await db.transaction(async (tx) => {
713+
async function forfeitFreeTierAllowance(executor: DbOrTx, userId: string): Promise<void> {
714+
await executor.transaction(async (tx) => {
699715
await tx.update(userStats).set({ freeTierEligible: false }).where(eq(userStats.userId, userId))
700716
await syncUsageLimitsFromSubscription(userId, tx)
701717
})

‎apps/sim/lib/billing/organizations/membership.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
invalidateSecurityPolicyVersionCache,
3030
} from '@/lib/auth/security-policy'
3131
import { applySessionPolicyToNewMember } from '@/lib/auth/session-policy'
32-
import { syncUsageLimitsFromSubscription } from '@/lib/billing/core/usage'
32+
import { createUserStatsIfMissing, syncUsageLimitsFromSubscription } from '@/lib/billing/core/usage'
3333
import {
3434
assertNoUnresolvedEnterpriseIssuance,
3535
resolveEnterpriseMetadataIntent,
@@ -1935,14 +1935,7 @@ export async function transferOrganizationOwnership(
19351935
.limit(1)
19361936

19371937
if (oldStats) {
1938-
await tx
1939-
.insert(userStats)
1940-
.values({
1941-
id: generateId(),
1942-
userId: newOwnerUserId,
1943-
usageLimitUpdatedAt: new Date(),
1944-
})
1945-
.onConflictDoNothing({ target: userStats.userId })
1938+
await createUserStatsIfMissing(tx, newOwnerUserId)
19461939

19471940
const overage = oldStats.billedOverageThisPeriod || '0'
19481941
const overageNum = toNumber(toDecimal(overage))

‎apps/sim/lib/billing/storage/tracking.ts‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919

2020
import { organization, project, userStats, workspace } from '@sim/db/schema'
2121
import { createLogger } from '@sim/logger'
22-
import { generateId } from '@sim/utils/id'
2322
import { isRecordLike } from '@sim/utils/object'
2423
import { and, asc, eq, inArray, sql } from 'drizzle-orm'
2524
import { maybeNotifyLimit } from '@/lib/billing/core/limit-notifications'
2625
import type { HighestPrioritySubscription } from '@/lib/billing/core/plan'
26+
import { createUserStatsIfMissing } from '@/lib/billing/core/usage'
2727
import type { BillingEntity } from '@/lib/billing/core/usage-log'
2828
import type {
2929
ProjectStorageBillingContext,
@@ -39,7 +39,7 @@ import {
3939
isStorageEnforcementEnabled,
4040
StorageLimitExceededError,
4141
} from '@/lib/billing/storage/limits'
42-
import { getFreeTierLimit, isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils'
42+
import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils'
4343
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
4444
import { lockProject } from '@/lib/projects/membership'
4545

@@ -764,14 +764,12 @@ export async function incrementAdmittedStorageUsageForBillingContextInTx(
764764
* window between the read and the increment. This helper does not send
765765
* notifications because it runs mid-transaction.
766766
*
767-
* For a personal (non-org-scoped) `userId`, this first upserts the
768-
* `userStats` row on `tx` — a documented possibility for OAuth account
769-
* linking (see `ensureUserStatsExists` in `lib/billing/core/usage.ts`, whose
770-
* insert values this mirrors) — because the conditional `UPDATE` below
771-
* matches 0 rows, and therefore reads as "quota exceeded", if that row
772-
* doesn't exist yet. `ensureUserStatsExists` itself isn't reused here since
773-
* it writes through the standalone `db` client, which would open a second
774-
* pooled connection while this transaction's is held.
767+
* For a personal (non-org-scoped) `userId`, this first creates a missing
768+
* `userStats` row on `tx` with `createUserStatsIfMissing` — a documented
769+
* possibility for OAuth account linking — because the conditional `UPDATE`
770+
* below matches 0 rows, and therefore reads as "quota exceeded", if that row
771+
* doesn't exist yet. It runs on `tx` rather than the standalone `db` client,
772+
* which would open a second pooled connection while this transaction's is held.
775773
*/
776774
export async function checkAndIncrementStorageUsageInTx(
777775
tx: DbOrTx,
@@ -791,17 +789,7 @@ export async function checkAndIncrementStorageUsageInTx(
791789

792790
const orgScoped = isOrgScopedSubscription(sub, userId) && sub
793791

794-
if (!orgScoped) {
795-
await tx
796-
.insert(userStats)
797-
.values({
798-
id: generateId(),
799-
userId,
800-
currentUsageLimit: getFreeTierLimit().toString(),
801-
usageLimitUpdatedAt: new Date(),
802-
})
803-
.onConflictDoNothing({ target: userStats.userId })
804-
}
792+
if (!orgScoped) await createUserStatsIfMissing(tx, userId)
805793

806794
const [updated] = orgScoped
807795
? await tx

‎apps/sim/lib/core/admission/transient-failure.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,23 @@ function hasAdmissionCode(failure: AdmissionFailureLike, code: string): boolean
8383
return failure.code === code || failure.cause?.code === code
8484
}
8585

86+
const RESERVATION_HEADROOM_DESCRIPTORS = [
87+
ADMISSION_ERROR_DESCRIPTOR.RESERVATION_PAYER_HEADROOM,
88+
ADMISSION_ERROR_DESCRIPTOR.RESERVATION_MEMBER_HEADROOM,
89+
] as const
90+
91+
/**
92+
* Whether a reservation was denied because the payer's or member's usage headroom is spent. The
93+
* run is refused for good, though a later one can be admitted as in-flight runs settle, which is
94+
* why deterministic admission rejections leave these out.
95+
*/
96+
export function isReservationHeadroomDenial(failure: AdmissionFailureLike): boolean {
97+
return RESERVATION_HEADROOM_DESCRIPTORS.some(
98+
(descriptor) =>
99+
failure.statusCode === descriptor.statusCode && hasAdmissionCode(failure, descriptor.code)
100+
)
101+
}
102+
86103
/**
87104
* Identifies only admission failures that unattended callers can safely retry.
88105
*/

‎apps/sim/lib/core/async-jobs/types.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,9 +338,12 @@ export interface JobQueueBackend {
338338
cancelByKey(cancelKey: string): boolean
339339

340340
/**
341-
* Cancel every queued, delayed or running workflow job (`WORKSPACE_EXECUTION_JOB_TYPES`)
342-
* enqueued for a workspace, for when the workspace itself is being removed. Throws when any
343-
* listing or cancellation failed, after attempting the rest, so the caller can retry.
341+
* Cancel every queued, delayed or running persisted workflow job
342+
* (`WORKSPACE_EXECUTION_JOB_TYPES`) enqueued for a workspace, for when the workspace itself is
343+
* being removed. Runs the database backend executes through `batchEnqueueAndWait` keep no job
344+
* row, so this never reaches them; they stop through their execution's cancellation signal.
345+
* Throws when any listing or cancellation failed, after attempting the rest, so the caller can
346+
* retry.
344347
*/
345348
cancelByWorkspace(
346349
workspaceId: string,

0 commit comments

Comments
 (0)