Skip to content

Commit 37a6c33

Browse files
icecrasher321claude
andcommitted
fix(usage): repair the run-count predicate, align the ledger and analytics scopes
Fourth review round. The first item is a break this branch introduced last round. `getBillingPeriodWorkflowRunCount` was rewritten to exclude unbilled categories via `<> ALL(${UNBILLED_USAGE_CATEGORIES})`. Interpolating a JavaScript array into a `sql` template emits parenthesized scalar binds, so the statement rendered as `ALL(($1))` and Postgres rejects it: "op ANY/ALL (array) requires array on right side" — verified against a real database. Its only caller builds the enterprise billing preview, so that preview would have thrown on every request. It now uses `notInArray`. Unit tests could not have caught it; `@sim/db` is mocked, so no statement is ever rendered. The ledger listing filtered on `created_at` while the analytics scope matches a stripe or default period on the stamps rows carry. The event list and the CSV therefore covered a different set than the totals above them — rows created inside the period but stamped to another, and the reverse. Both now derive their filter from one `usageWindowLedgerFilter`, which mirrors `buildUsageAnalyticsScope` case for case, with a test asserting the two branch on the same discriminant. An invalid timezone reached `assertValidTimezone` and surfaced as a 500 for what is an ordinary bad query param; it is now refused by the contract as a 400, with the SQL-boundary assertion left in place as the backstop it is. A bookmarked workspace id that no longer resolves opened a detail view with an untitled header and empty sections, against this repo's own deep-link rule. It now falls back to the list once the list has loaded. Also: `Cache-Control: no-store` on the CSV, which is every member's spend behind session auth and the one response a browser will cache; Export no longer gated on an unrelated summary query; the bar chart keeps its measurement ref on the empty branch; and the admin create-organization contract declares `attachedWorkspaceIds` plus `workspaceAttachmentFailed`, so a caller can tell an owner with no workspaces from provisioning that is genuinely incomplete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b227717 commit 37a6c33

15 files changed

Lines changed: 20524 additions & 41 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ export const GET = withRouteHandler(async (request: NextRequest, context) => {
7171
return new NextResponse(csv, {
7272
headers: {
7373
'Content-Type': 'text/csv; charset=utf-8',
74+
// Every member's spend for one organization, behind session auth. Without
75+
// this a browser or shared intermediary may serve it again after the
76+
// viewer's access to that organization has been revoked.
77+
'Cache-Control': 'no-store',
7478
'Content-Disposition': `attachment; filename="organization-usage-${params.id}.csv"`,
7579
...(result.truncated ? { 'X-Export-Truncated': '1' } : {}),
7680
},

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
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; attachedWorkspaceIds: string[] }>
21+
* Response: AdminSingleResponse<AdminOrganization & { memberId, attachedWorkspaceIds, workspaceAttachmentFailed }>
2222
* `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.
23+
* organization, and `workspaceAttachmentFailed` distinguishes an owner who had
24+
* none from an attachment that threw after the organization was committed — the
25+
* latter leaves the organization unreachable and is worth retrying.
2526
*/
2627

2728
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
@@ -178,11 +179,11 @@ export const POST = withRouteHandler(
178179
* blocked by the existing-membership check above, with no way to reach the
179180
* organization at all.
180181
*
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.
182+
* It must not be reported as an unqualified success either. Both the ids that
183+
* moved and whether the attach threw are on the response contract and on the
184+
* audit event, because an empty list alone cannot distinguish an owner who had
185+
* no workspaces from provisioning that is genuinely incomplete — only the
186+
* second is worth retrying, and only the caller can decide to.
186187
*/
187188
let attachedWorkspaceIds: string[] = []
188189
let workspaceAttachmentFailed = false
@@ -238,6 +239,7 @@ export const POST = withRouteHandler(
238239
...toAdminOrganization(createdOrg),
239240
memberId,
240241
attachedWorkspaceIds,
242+
workspaceAttachmentFailed,
241243
})
242244
} catch (error) {
243245
if (error instanceof OrganizationSlugInvalidError) {

apps/sim/components/charts/bar-chart.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,11 @@ function BarChartComponent({
133133

134134
if (data.length === 0) {
135135
return (
136+
// Keeps the measurement ref: dropping it here left the observer watching a
137+
// detached node, so a resize while empty was never seen and the next non-empty
138+
// render laid out at the stale width.
136139
<div
140+
ref={containerRef}
137141
className={cn(
138142
'flex items-center justify-center',
139143
!hasExternalWrapper && 'rounded-lg border bg-card p-4'

apps/sim/ee/organization-usage/components/usage-monitoring.tsx

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,15 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring
9393
const [creditsTarget, setCreditsTarget] = useState<ManageCreditsTarget | null>(null)
9494

9595
const isOverview = tab === USAGE_OVERVIEW_TAB
96-
/** A selected workspace turns the Workspaces tab into that workspace's workflows. */
97-
const isWorkspaceDetail = tab === 'workspace' && Boolean(workspace)
98-
const dimension: UsageBreakdownDimension = isOverview
99-
? 'source'
100-
: isWorkspaceDetail
101-
? 'workflow'
102-
: (tab as UsageBreakdownDimension)
96+
/**
97+
* A selected workspace turns the Workspaces tab into that workspace's workflows —
98+
* but only once the id resolves against the loaded list. A bookmarked id for a
99+
* deleted workspace, or one belonging to another organization, would otherwise open
100+
* a detail view with an untitled header and empty sections. Falling back to the
101+
* list is the rule for every deep-linked entity id (`sim-url-state.md`); the
102+
* lingering param is harmless.
103+
*/
104+
const isWorkspaceSelected = tab === 'workspace' && Boolean(workspace)
103105

104106
/**
105107
* Per-member caps are hosted-only: the usage-limit route 404s where Sim does not
@@ -111,18 +113,31 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring
111113
const canManageCredits = tab === 'member' && isHosted
112114

113115
const summary = useOrganizationUsageSummary(organizationId, window)
114-
const breakdown = useOrganizationUsageBreakdown(organizationId, window, dimension, {
115-
...(isWorkspaceDetail && workspace ? { workspaceId: workspace } : {}),
116-
})
117116
/**
118117
* Kept alive in the drill-down purely to name it. The rule is to store the id and
119118
* derive the entity from the loaded list; arriving by click serves this from cache,
120119
* and arriving by deep link fetches it once.
121120
*/
122121
const workspaceList = useOrganizationUsageBreakdown(organizationId, window, 'workspace', {
123-
enabled: isWorkspaceDetail,
122+
enabled: isWorkspaceSelected,
124123
})
125124
const workspaceName = workspaceList.data?.rows.find((row) => row.id === workspace)?.label
125+
/**
126+
* Resolved, not merely present — and only once the list has actually loaded, so a
127+
* deep link does not flash the Workspaces tab before its own detail view.
128+
*/
129+
const isWorkspaceDetail =
130+
isWorkspaceSelected && (workspaceList.isLoading || workspaceName !== undefined)
131+
132+
const dimension: UsageBreakdownDimension = isOverview
133+
? 'source'
134+
: isWorkspaceDetail
135+
? 'workflow'
136+
: (tab as UsageBreakdownDimension)
137+
138+
const breakdown = useOrganizationUsageBreakdown(organizationId, window, dimension, {
139+
...(isWorkspaceDetail && workspace ? { workspaceId: workspace } : {}),
140+
})
126141
const workspaceSources = useOrganizationUsageBreakdown(organizationId, window, 'source', {
127142
enabled: isWorkspaceDetail,
128143
...(workspace ? { workspaceId: workspace } : {}),
@@ -247,7 +262,6 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring
247262
text: 'Export',
248263
icon: Download,
249264
onSelect: () => void handleExport(),
250-
disabled: summary.isLoading || summary.isError,
251265
},
252266
]}
253267
>

apps/sim/lib/api/contracts/organization-usage.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from 'zod'
22
import { organizationIdSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives'
33
import { defineRouteContract } from '@/lib/api/contracts/types'
44
import { INTERNAL_USAGE_LOG_SOURCES } from '@/lib/billing/usage-sources'
5+
import { isValidTimezone } from '@/lib/core/utils/timezone'
56

67
/**
78
* Organization usage monitoring (enterprise).
@@ -55,8 +56,20 @@ const organizationUsageWindowQuerySchema = z.object({
5556
preset: usageWindowPresetSchema,
5657
startDate: isoDateSchema,
5758
endDate: isoDateSchema,
58-
/** IANA name; bucket boundaries are the viewer's calendar days. */
59-
timezone: z.string().min(1, 'timezone cannot be empty').default('UTC'),
59+
/**
60+
* IANA name; bucket boundaries are the viewer's calendar days.
61+
*
62+
* Validated here rather than only at the SQL boundary. `assertValidTimezone` is a
63+
* hard gate — the value reaches `AT TIME ZONE`, which takes an identifier and not
64+
* a bound parameter — but it throws a plain `Error`, which surfaced as a 500 for
65+
* what is an ordinary bad query param. Rejecting it as a contract violation makes
66+
* it a 400 with a usable message and leaves that gate as the backstop it is.
67+
*/
68+
timezone: z
69+
.string()
70+
.min(1, 'timezone cannot be empty')
71+
.refine(isValidTimezone, 'Expected an IANA timezone such as America/Los_Angeles')
72+
.default('UTC'),
6073
})
6174

6275
export const organizationUsageSummaryQuerySchema = organizationUsageWindowQuerySchema

apps/sim/lib/api/contracts/v1/admin/organizations.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,31 @@ export const adminV1ListOrganizationsContract = defineRouteContract({
209209
},
210210
})
211211

212+
/**
213+
* Creating an organization also attaches the owner's existing workspaces, so that
214+
* the organization is reachable through the workspace-scoped settings it is
215+
* administered from. That attachment runs in its own transaction and can fail after
216+
* the organization is already committed, so the outcome is part of the response
217+
* rather than something only the logs know.
218+
*
219+
* `attachedWorkspaceIds` alone is ambiguous — empty means both "the owner had none"
220+
* and "the attach threw" — which is why the failure carries its own flag. A caller
221+
* seeing `workspaceAttachmentFailed` can retry the attachment; a caller seeing an
222+
* empty list without it has nothing outstanding.
223+
*/
224+
const adminV1CreatedOrganizationSchema = adminV1OrganizationSchema.extend({
225+
memberId: z.string(),
226+
attachedWorkspaceIds: z.array(z.string()),
227+
workspaceAttachmentFailed: z.boolean(),
228+
})
229+
212230
export const adminV1CreateOrganizationContract = defineRouteContract({
213231
method: 'POST',
214232
path: '/api/v1/admin/organizations',
215233
body: adminV1CreateOrganizationBodySchema,
216234
response: {
217235
mode: 'json',
218-
schema: adminV1SingleResponseSchema(adminV1OrganizationSchema.extend({ memberId: z.string() })),
236+
schema: adminV1SingleResponseSchema(adminV1CreatedOrganizationSchema),
219237
},
220238
})
221239

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { organizationUsageOperations } from '@/lib/billing/application/organizat
33
import {
44
resolveUsageAnalyticsWindow,
55
type UsageWindowPreset,
6-
usageWindowBounds,
6+
usageWindowLedgerFilter,
77
} from '@/lib/billing/core/usage-analytics'
88
import { getBillingEntityUsageLogs } from '@/lib/billing/core/usage-log'
99
import { dollarsToCredits } from '@/lib/billing/credits/conversion'
@@ -56,18 +56,17 @@ export const exportOrganizationUsageEvents = defineAuthorizedOrganizationUsageUs
5656
customEnd: input.endDate,
5757
timezone: input.timezone,
5858
})
59-
const bounds = usageWindowBounds(window)
59+
const ledgerFilter = usageWindowLedgerFilter(window)
6060

6161
const rows: OrganizationUsageExportRow[] = []
6262
let cursor: string | undefined
6363
let truncated = false
6464

6565
while (rows.length < USAGE_EXPORT_SAFETY_CAP) {
6666
const page = await getBillingEntityUsageLogs(context.billingEntity, {
67-
startDate: bounds.start,
68-
endDate: bounds.end,
69-
// Half-open, matching the summary and breakdowns — see `endDateExclusive`.
70-
endDateExclusive: true,
67+
// One derivation for both predicates, so the CSV covers exactly the rows the
68+
// summary and breakdowns aggregate over.
69+
...ledgerFilter,
7170
...(input.source?.length ? { source: input.source } : {}),
7271
limit: EXPORT_PAGE_SIZE,
7372
...(cursor ? { cursor } : {}),

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { organizationUsageOperations } from '@/lib/billing/application/organizat
33
import {
44
resolveUsageAnalyticsWindow,
55
type UsageWindowPreset,
6-
usageWindowBounds,
6+
usageWindowLedgerFilter,
77
} from '@/lib/billing/core/usage-analytics'
88
import { getBillingEntityUsageLogs } from '@/lib/billing/core/usage-log'
99
import { dollarsToCredits } from '@/lib/billing/credits/conversion'
@@ -57,13 +57,10 @@ export const listOrganizationUsageEvents = defineAuthorizedOrganizationUsageUseC
5757
customEnd: input.endDate,
5858
timezone: input.timezone,
5959
})
60-
const bounds = usageWindowBounds(window)
61-
6260
const result = await getBillingEntityUsageLogs(context.billingEntity, {
63-
startDate: bounds.start,
64-
endDate: bounds.end,
65-
// Half-open, matching the summary and breakdowns — see `endDateExclusive`.
66-
endDateExclusive: true,
61+
// One derivation for both predicates, so this list covers exactly the rows the
62+
// summary and breakdowns aggregate over.
63+
...usageWindowLedgerFilter(window),
6764
...(input.source?.length ? { source: input.source } : {}),
6865
limit: input.limit,
6966
...(input.cursor ? { cursor: input.cursor } : {}),

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
resolveUsageBucket,
1515
UsageWindowRangeInvertedError,
1616
UsageWindowRangeTooLargeError,
17+
usageWindowLedgerFilter,
1718
} from '@/lib/billing/core/usage-analytics'
1819

1920
const ENTITY = { type: 'organization', id: 'org-1' } as const
@@ -69,6 +70,49 @@ describe('buildUsageAnalyticsScope', () => {
6970
})
7071
})
7172

73+
describe('usageWindowLedgerFilter', () => {
74+
it('matches a stripe period on the stamps, as the aggregate does', () => {
75+
// Filtering this window on `created_at` instead selects a different set — rows
76+
// created inside the period but stamped to another, and vice versa — so the event
77+
// list and the CSV covered different rows than the totals above them.
78+
const filter = usageWindowLedgerFilter({ kind: 'period', period: period({ source: 'stripe' }) })
79+
expect(filter).toEqual({
80+
billingPeriod: {
81+
start: new Date('2026-08-01T00:00:00.000Z'),
82+
end: new Date('2026-09-01T00:00:00.000Z'),
83+
},
84+
})
85+
})
86+
87+
it('matches a reporting period on created_at, as the aggregate does', () => {
88+
const filter = usageWindowLedgerFilter({
89+
kind: 'period',
90+
period: period({ source: 'reporting', anchorDate: '2026-08-01', interval: 'month' }),
91+
})
92+
expect(filter.billingPeriod).toBeUndefined()
93+
expect(filter.endDateExclusive).toBe(true)
94+
})
95+
96+
it('keeps a plain range half-open', () => {
97+
const from = new Date('2026-08-01T00:00:00.000Z')
98+
const to = new Date('2026-08-08T00:00:00.000Z')
99+
expect(usageWindowLedgerFilter({ kind: 'range', from, to })).toEqual({
100+
startDate: from,
101+
endDate: to,
102+
endDateExclusive: true,
103+
})
104+
})
105+
106+
it('branches on the same condition the scope builder does', () => {
107+
// The two predicates are only in step because they read the same discriminant.
108+
for (const source of ['reporting', 'stripe', 'default'] as const) {
109+
const window = { kind: 'period' as const, period: period({ source }) }
110+
const usesStamps = scopeShape(window).includes('usageLog.billingPeriodStart')
111+
expect(usageWindowLedgerFilter(window).billingPeriod !== undefined).toBe(usesStamps)
112+
}
113+
})
114+
})
115+
72116
describe('resolveUsageAnalyticsWindow', () => {
73117
const now = new Date('2026-08-20T12:00:00.000Z')
74118

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,36 @@ export function usageWindowBounds(window: UsageAnalyticsWindow): { start: Date;
9999
: { start: window.period.start, end: window.period.end }
100100
}
101101

102+
export interface UsageWindowLedgerFilter {
103+
startDate?: Date
104+
endDate?: Date
105+
endDateExclusive?: boolean
106+
billingPeriod?: { start: Date; end: Date }
107+
}
108+
109+
/**
110+
* The same window, expressed for the ledger listing query.
111+
*
112+
* `getBillingEntityUsageLogs` filters rows while {@link buildUsageAnalyticsScope}
113+
* aggregates them, and the two must select the same set or the event list and CSV
114+
* describe different rows than the totals above them. That is not hypothetical for a
115+
* stripe or default period: those are matched on the *stamps* rows carry, and
116+
* filtering the same window on `created_at` picks up rows created inside it but
117+
* stamped to another period, while missing the reverse.
118+
*
119+
* Deriving both from one function is what keeps the predicates in step — the branches
120+
* below mirror `buildUsageAnalyticsScope` case for case.
121+
*/
122+
export function usageWindowLedgerFilter(window: UsageAnalyticsWindow): UsageWindowLedgerFilter {
123+
if (window.kind === 'range' || window.period.source === 'reporting') {
124+
const { start, end } = usageWindowBounds(window)
125+
// Half-open, so a row on the boundary belongs to the next window — as it does
126+
// for the aggregate, whose `lt` says the same thing.
127+
return { startDate: start, endDate: end, endDateExclusive: true }
128+
}
129+
return { billingPeriod: { start: window.period.start, end: window.period.end } }
130+
}
131+
102132
export class UsageWindowRangeTooLargeError extends Error {
103133
constructor(days: number) {
104134
super(`Custom range spans ${days} days; the maximum is ${MAX_CUSTOM_RANGE_DAYS}.`)

0 commit comments

Comments
 (0)