Skip to content

Commit 45e8c0a

Browse files
icecrasher321claude
andcommitted
fix(usage): resolve custom bounds in the viewer calendar, bound the open period
Third review round. Custom ranges were anchored on UTC instants. The picker offers calendar days and sends `YYYY-MM-DD`, which arrives parsed as UTC midnight, so every non-UTC viewer's selection was shifted by their offset — a range labelled "Aug 1–31" covered half of Jul 31 and half of Aug 31 twelve hours east — and it contradicted the series, whose buckets are already the viewer's calendar days. The resolver now takes the timezone and reinterprets the same civil dates as midnight there, through `zonedWallClockToUtc`, and counts the span in civil days so a range containing a DST transition is not measured as 91.96. The timezone is threaded through the breakdown, events, and export inputs as well; the contract already carried it but only the summary consumed it, so the four surfaces would have resolved one range four ways. A deployment with no subscription resolves to `defaultBillingPeriod()`, the open pair 1970…9999. Rendered as a period that produced a thousand monthly buckets ending in 2053, stopped only by the densifier's loop guard — measured, not inferred. Self-hosted is exactly where it is reachable, since the usage flag opens the panel on deployments with no plan at all. An unbounded period now shows a rolling 30-day window, and its predecessor steps back by that window rather than by a span of eight millennia. Kept as one window rather than clamping only the chart, so the series still sums to the headline. The summary's delta used the `previous-period` preset, which must always return something and therefore approximates a stripe period's predecessor by stepping back the current period's length. Stripe periods are not equal-length, so the comparison could be measured against a window that is not the previous period — contradicting the comment directly above it. It now calls `resolvePreviousPeriod` and shows no delta when there is no exact predecessor. Reversed custom bounds measured a negative span, passed the cap, and returned an inverted range matching nothing — "no usage" rather than a bad request. They now throw, classified 400 alongside the too-large error. Admin organization provisioning reported an unqualified success when workspace attachment failed. It now returns `attachedWorkspaceIds` and records it on the audit event, so the incomplete state is visible to the caller and durable after the fact, rather than known only to the logs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 54e3f27 commit 45e8c0a

11 files changed

Lines changed: 232 additions & 34 deletions

File tree

apps/sim/app/api/organizations/[id]/usage/breakdown/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export const GET = defineInternalJsonRoute({
3131
preset: query.preset,
3232
startDate: query.startDate ? new Date(query.startDate) : undefined,
3333
endDate: query.endDate ? new Date(query.endDate) : undefined,
34+
timezone: query.timezone,
3435
limit: query.limit,
3536
}),
3637
useCase: getOrganizationUsageBreakdown,

apps/sim/app/api/organizations/[id]/usage/error-policy.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@ import {
33
internalErrorResponse,
44
internalOrchestrationErrorPolicy,
55
} from '@/lib/api/server/routes'
6-
import { UsageWindowRangeTooLargeError } from '@/lib/billing/core/usage-analytics'
6+
import {
7+
UsageWindowRangeInvertedError,
8+
UsageWindowRangeTooLargeError,
9+
} from '@/lib/billing/core/usage-analytics'
710

811
/**
9-
* The window resolver throws when a custom range exceeds its cap, which is a
12+
* The window resolver throws when a custom range exceeds its cap or ends before it
13+
begins, both of which are
1014
* caller-fixable input error rather than a fault. Without this it fell through to
1115
* the orchestration policy's `unhandled` branch and every over-long range answered
1216
* `500 Internal server error`, so the client could neither surface the real reason
@@ -17,7 +21,7 @@ import { UsageWindowRangeTooLargeError } from '@/lib/billing/core/usage-analytic
1721
export const organizationUsageErrorPolicy = extendInternalErrorPolicy(
1822
internalOrchestrationErrorPolicy,
1923
(error) =>
20-
error instanceof UsageWindowRangeTooLargeError
24+
error instanceof UsageWindowRangeTooLargeError || error instanceof UsageWindowRangeInvertedError
2125
? internalErrorResponse(400, { error: error.message })
2226
: null
2327
)

apps/sim/app/api/organizations/[id]/usage/events/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export const GET = defineInternalJsonRoute({
2929
preset: query.preset,
3030
startDate: query.startDate ? new Date(query.startDate) : undefined,
3131
endDate: query.endDate ? new Date(query.endDate) : undefined,
32+
timezone: query.timezone,
3233
source: query.source,
3334
limit: query.limit,
3435
cursor: query.cursor,

apps/sim/app/api/organizations/[id]/usage/export/route.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
exportOrganizationUsageEvents,
99
type OrganizationUsageExportRow,
1010
} from '@/lib/billing/application/organization-usage/export-organization-usage-events'
11-
import { UsageWindowRangeTooLargeError } from '@/lib/billing/core/usage-analytics'
11+
import {
12+
UsageWindowRangeInvertedError,
13+
UsageWindowRangeTooLargeError,
14+
} from '@/lib/billing/core/usage-analytics'
1215
import { formatCreditsLabel } from '@/lib/billing/credits/conversion'
1316
import { ForbiddenOperationError } from '@/lib/core/application'
1417
import { formatCsvValue, toCsvRow } from '@/lib/core/utils/csv'
@@ -59,6 +62,7 @@ export const GET = withRouteHandler(async (request: NextRequest, context) => {
5962
preset: query.preset,
6063
startDate: query.startDate ? new Date(query.startDate) : undefined,
6164
endDate: query.endDate ? new Date(query.endDate) : undefined,
65+
timezone: query.timezone,
6266
source: query.source,
6367
},
6468
})
@@ -75,9 +79,12 @@ export const GET = withRouteHandler(async (request: NextRequest, context) => {
7579
if (error instanceof ForbiddenOperationError) {
7680
return NextResponse.json({ error: error.message }, { status: 403 })
7781
}
78-
// A range over the cap is the caller's input, not a fault — the same
82+
// A range over the cap, or inverted, is the caller's input — the same
7983
// classification the three JSON routes make through `organizationUsageErrorPolicy`.
80-
if (error instanceof UsageWindowRangeTooLargeError) {
84+
if (
85+
error instanceof UsageWindowRangeTooLargeError ||
86+
error instanceof UsageWindowRangeInvertedError
87+
) {
8188
return NextResponse.json({ error: error.message }, { status: 400 })
8289
}
8390
logger.error('Failed to export organization usage', { error: getErrorMessage(error) })

apps/sim/app/api/v1/admin/organizations/route.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
* - slug: string - Organization slug (optional, auto-generated from name if not provided)
1919
* - ownerId: string - User ID of the organization owner (required)
2020
*
21-
* Response: AdminSingleResponse<AdminOrganization & { memberId: string }>
21+
* Response: AdminSingleResponse<AdminOrganization & { memberId: string; attachedWorkspaceIds: string[] }>
22+
* `attachedWorkspaceIds` reports which of the owner's workspaces moved under the
23+
* organization. Empty means none did — either the owner had none, or attachment
24+
* failed after the organization was already committed.
2225
*/
2326

2427
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
@@ -173,10 +176,16 @@ export const POST = withRouteHandler(
173176
* So its failure must not be reported as a failure to create: the organization
174177
* is already committed, and answering 500 for state that exists left the retry
175178
* blocked by the existing-membership check above, with no way to reach the
176-
* organization at all. Log it and return the organization that was created —
177-
* attaching a workspace afterwards is a normal, repeatable operation.
179+
* organization at all.
180+
*
181+
* It must not be reported as an unqualified success either. The outcome is
182+
* returned as `attachedWorkspaceIds` and recorded on the audit event, so a
183+
* caller sees which workspaces moved — an empty array after a failure is the
184+
* explicit incomplete state, and attaching afterwards is a normal repeatable
185+
* operation rather than something only the logs know is outstanding.
178186
*/
179187
let attachedWorkspaceIds: string[] = []
188+
let workspaceAttachmentFailed = false
180189
try {
181190
;({ attachedWorkspaceIds } = await attachOwnedWorkspacesToOrganization({
182191
ownerUserId: ownerId,
@@ -185,6 +194,7 @@ export const POST = withRouteHandler(
185194
includeArchived: true,
186195
}))
187196
} catch (attachError) {
197+
workspaceAttachmentFailed = true
188198
logger.error('Admin API: Created organization but could not attach its workspaces', {
189199
organizationId,
190200
ownerId,
@@ -214,13 +224,20 @@ export const POST = withRouteHandler(
214224
resourceId: organizationId,
215225
resourceName: name,
216226
description: `Admin API created organization "${name}"`,
217-
metadata: { slug, ownerId, memberId },
227+
metadata: {
228+
slug,
229+
ownerId,
230+
memberId,
231+
attachedWorkspaceIds,
232+
...(workspaceAttachmentFailed ? { workspaceAttachmentFailed: true } : {}),
233+
},
218234
request,
219235
})
220236

221237
return singleResponse({
222238
...toAdminOrganization(createdOrg),
223239
memberId,
240+
attachedWorkspaceIds,
224241
})
225242
} catch (error) {
226243
if (error instanceof OrganizationSlugInvalidError) {

apps/sim/lib/billing/application/organization-usage/export-organization-usage-events.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export interface OrganizationUsageExportInput {
2121
preset: UsageWindowPreset
2222
startDate?: Date
2323
endDate?: Date
24+
/** Viewer calendar, so a date-only custom bound means midnight there. */
25+
timezone?: string
2426
source?: InternalUsageLogSource[]
2527
}
2628

@@ -52,6 +54,7 @@ export const exportOrganizationUsageEvents = defineAuthorizedOrganizationUsageUs
5254
period: context.period,
5355
customStart: input.startDate,
5456
customEnd: input.endDate,
57+
timezone: input.timezone,
5558
})
5659
const bounds = usageWindowBounds(window)
5760

apps/sim/lib/billing/application/organization-usage/get-organization-usage-breakdown.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export interface OrganizationUsageBreakdownInput {
2828
preset: UsageWindowPreset
2929
startDate?: Date
3030
endDate?: Date
31+
/** Viewer calendar, so a date-only custom bound means midnight there. */
32+
timezone?: string
3133
/** Narrows to one workspace, for the Workspaces drill-down. */
3234
workspaceId?: string
3335
limit: number
@@ -81,6 +83,7 @@ export const getOrganizationUsageBreakdown = defineAuthorizedOrganizationUsageUs
8183
period: context.period,
8284
customStart: input.startDate,
8385
customEnd: input.endDate,
86+
timezone: input.timezone,
8487
})
8588
const scope = buildUsageAnalyticsScope(context.billingEntity, window)
8689
const raw = await readUsageBreakdown(scope, input.dimension, input.workspaceId)

apps/sim/lib/billing/application/organization-usage/get-organization-usage-summary.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { UsagePeriodSource } from '@/lib/billing/core/reporting-period'
44
import {
55
buildUsageAnalyticsScope,
66
densifyUsageSeries,
7+
resolvePreviousPeriod,
78
resolveUsageAnalyticsWindow,
89
resolveUsageBucket,
910
type UsageBucket,
@@ -43,19 +44,25 @@ export const getOrganizationUsageSummary = defineAuthorizedOrganizationUsageUseC
4344
period: context.period,
4445
customStart: input.startDate,
4546
customEnd: input.endDate,
47+
timezone: input.timezone,
4648
})
4749
const bucket = resolveUsageBucket(window)
4850
const scope = buildUsageAnalyticsScope(context.billingEntity, window)
4951

5052
/**
51-
* The comparison window only exists when it is exactly derivable. A stripe period
52-
* has no rule for its predecessor, and no delta beats a delta measured against the
53-
* wrong window.
53+
* The comparison window only exists when it is exactly derivable.
54+
*
55+
* `resolvePreviousPeriod` directly, not the `previous-period` preset: that preset
56+
* must always return *something*, because a user who explicitly asks for the
57+
* previous period has to see a window — so for a stripe period it approximates one
58+
* by stepping back the current period's length. That approximation is fine as a
59+
* destination and wrong as a baseline, since Stripe periods are not equal-length
60+
* and the delta would silently compare against a window that is not the previous
61+
* period. No delta beats a delta measured against the wrong window.
5462
*/
55-
const comparison =
56-
input.preset === 'current-period'
57-
? resolveUsageAnalyticsWindow({ preset: 'previous-period', period: context.period })
58-
: null
63+
const previousPeriod =
64+
input.preset === 'current-period' ? resolvePreviousPeriod(context.period) : null
65+
const comparison = previousPeriod ? { kind: 'period' as const, period: previousPeriod } : null
5966

6067
const [totals, seriesRows, previous] = await Promise.all([
6168
readUsageTotals(scope),

apps/sim/lib/billing/application/organization-usage/list-organization-usage-events.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ export interface OrganizationUsageEventsInput {
1414
preset: UsageWindowPreset
1515
startDate?: Date
1616
endDate?: Date
17+
/** Viewer calendar, so a date-only custom bound means midnight there. */
18+
timezone?: string
1719
source?: InternalUsageLogSource[]
1820
limit: number
1921
cursor?: string
@@ -53,6 +55,7 @@ export const listOrganizationUsageEvents = defineAuthorizedOrganizationUsageUseC
5355
period: context.period,
5456
customStart: input.startDate,
5557
customEnd: input.endDate,
58+
timezone: input.timezone,
5659
})
5760
const bounds = usageWindowBounds(window)
5861

apps/sim/lib/billing/core/usage-analytics.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
resolvePreviousPeriod,
1313
resolveUsageAnalyticsWindow,
1414
resolveUsageBucket,
15+
UsageWindowRangeInvertedError,
1516
UsageWindowRangeTooLargeError,
1617
} from '@/lib/billing/core/usage-analytics'
1718

@@ -162,6 +163,77 @@ describe('resolveUsageAnalyticsWindow', () => {
162163
const window = resolveUsageAnalyticsWindow({ preset: 'custom', period: period(), now })
163164
expect(window.kind).toBe('period')
164165
})
166+
167+
it('anchors custom bounds on midnight in the viewer calendar, not UTC', () => {
168+
// The picker offers calendar days. Anchoring on the UTC instant shifted every
169+
// non-UTC viewer's selection by their offset, and disagreed with the chart,
170+
// whose buckets are already the viewer's calendar days.
171+
const window = resolveUsageAnalyticsWindow({
172+
preset: 'custom',
173+
period: period(),
174+
customStart: new Date('2026-08-01'),
175+
customEnd: new Date('2026-08-31'),
176+
timezone: 'America/New_York',
177+
now,
178+
})
179+
expect(window.kind).toBe('range')
180+
if (window.kind === 'range') {
181+
// Midnight on Aug 1 in New York is 04:00 UTC (EDT, UTC-4).
182+
expect(window.from.toISOString()).toBe('2026-08-01T04:00:00.000Z')
183+
expect(window.to.toISOString()).toBe('2026-09-01T04:00:00.000Z')
184+
}
185+
})
186+
187+
it('refuses a range that ends before it starts', () => {
188+
// Inverted bounds measured a negative span, passed the cap check, and produced a
189+
// range that matched nothing — indistinguishable from "no usage".
190+
expect(() =>
191+
resolveUsageAnalyticsWindow({
192+
preset: 'custom',
193+
period: period(),
194+
customStart: new Date('2026-08-31'),
195+
customEnd: new Date('2026-08-01'),
196+
now,
197+
})
198+
).toThrow(UsageWindowRangeInvertedError)
199+
})
200+
201+
it('shows a bounded window for a deployment with no subscription', () => {
202+
// `defaultBillingPeriod()` is the open pair 1970…9999. Rendered as a period it
203+
// produced 1,000 monthly buckets ending in 2053, stopped only by the densifier's
204+
// loop guard — reachable on self-hosted, where the panel opens without a plan.
205+
const window = resolveUsageAnalyticsWindow({
206+
preset: 'current-period',
207+
period: period({
208+
source: 'default',
209+
start: new Date(0),
210+
end: new Date(Date.UTC(9999, 11, 31)),
211+
}),
212+
now,
213+
})
214+
expect(window.kind).toBe('range')
215+
if (window.kind === 'range') {
216+
expect(window.to).toEqual(now)
217+
expect(Math.round((window.to.getTime() - window.from.getTime()) / 86_400_000)).toBe(30)
218+
}
219+
})
220+
221+
it('steps an unbounded period back by the display window, not by its own length', () => {
222+
const window = resolveUsageAnalyticsWindow({
223+
preset: 'previous-period',
224+
period: period({
225+
source: 'default',
226+
start: new Date(0),
227+
end: new Date(Date.UTC(9999, 11, 31)),
228+
}),
229+
now,
230+
})
231+
expect(window.kind).toBe('range')
232+
if (window.kind === 'range') {
233+
// Not 1970-minus-eight-millennia, which is what deriving it from the span gave.
234+
expect(window.from.getUTCFullYear()).toBe(2026)
235+
}
236+
})
165237
})
166238

167239
describe('resolvePreviousPeriod', () => {

0 commit comments

Comments
 (0)