Skip to content

Commit ca2d0bb

Browse files
committed
fix(v2): honour a requested stats window, and answer a claimed graph id with a conflict
Log statistics accepted a start and an end, filtered the totals by them, and then built the series against wall-clock now. Bucket width was computed over a span the caller never asked for, and every bucket past the requested end was structurally empty - so a bounded historical query returned a wrong-width series with fabricated trailing buckets, under a window label that disagreed with the request. Each edge now honours the bound it was given and keeps its previous derivation when omitted, so an unbounded request is unchanged. Separately, block, edge and subflow ids are global primary keys while the delete that precedes a state replace is scoped to one workflow. An id owned by another workflow survived that delete, the insert violated the key, and because callers pass their own transaction the driver error escaped unclassified as a server fault. The write now refuses such an id up front with a conflict naming it, and re-classifies the same violation if one races past the check, since the lock covers only the workflow being written. The dry run checks the ids a commit would insert and reports the warnings a commit would report, which is what its own contract already promised.
1 parent fe1db96 commit ca2d0bb

11 files changed

Lines changed: 562 additions & 25 deletions

File tree

apps/sim/app/api/logs/stats/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
7272
const whereCondition = commonFilters ? and(workspaceFilter, commonFilters) : workspaceFilter
7373

7474
const bounds = await readLogStatsBounds(whereCondition)
75-
const window = resolveLogStatsWindow(bounds, params.segmentCount)
75+
const window = resolveLogStatsWindow(bounds, params.segmentCount, {
76+
requestedStart: params.startDate ? new Date(params.startDate) : undefined,
77+
requestedEnd: params.endDate ? new Date(params.endDate) : undefined,
78+
})
7679
const rows = await readLogStatsSegments(
7780
whereCondition,
7881
window.startTime.toISOString(),

apps/sim/lib/api/contracts/v2/logs-stats.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export const v2LogStatsSchema = z
105105
end: v2TimestampSchema.describe('ISO 8601 end of the window.'),
106106
})
107107
.describe(
108-
'The window the buckets span: the oldest matching run through the later of the newest matching run and now. A workspace with no matching runs reports the trailing 24 hours.'
108+
'The window the buckets span. `startDate` and `endDate` are used verbatim when supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs the window is 24 hours wide, measured back from that right edge — the trailing 24 hours when no `endDate` was supplied, and the 24 hours preceding `endDate` when one was supplied without a `startDate`.'
109109
),
110110
segmentMs: z.number().describe('Width of one bucket in milliseconds.'),
111111
})

apps/sim/lib/api/contracts/v2/openapi/logs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ const declaredRoutes = [
207207
logsOperation({
208208
operationId: 'getLogStats',
209209
summary: 'Get Log Statistics',
210-
description: `Bucketed run counts, success rate, error count, and mean latency for a workspace and for each of its workflows — the aggregate a caller would otherwise have to page every run to compute. The window spans the oldest matching run through the later of the newest matching run and now, divided into exactly \`segmentCount\` equal buckets whose width is \`max(60000, floor(windowMs / segmentCount))\` milliseconds. The one-minute floor is a floor on bucket width, not on the window: when it applies, the series runs past \`timeBounds.end\` and the trailing buckets are empty rather than the window being compressed. A folder path covers its whole subtree. Per-workflow series are capped and \`workflowsTruncated\` reports whether the cap applied; the workspace totals are always computed from every workflow. ${RUN_RETENTION} ${FOLDER_TREE_TOO_LARGE}`,
210+
description: `Bucketed run counts, success rate, error count, and mean latency for a workspace and for each of its workflows — the aggregate a caller would otherwise have to page every run to compute. The window spans \`startDate\` through \`endDate\` when both are supplied; an omitted edge falls back to the oldest matching run on the left and to the later of the newest matching run and now on the right. With no matching runs the window is 24 hours wide, measured back from that right edge — the trailing 24 hours when no \`endDate\` was supplied, and the 24 hours preceding \`endDate\` when one was supplied without a \`startDate\`. The window is divided into exactly \`segmentCount\` equal buckets whose width is \`max(60000, floor(windowMs / segmentCount))\` milliseconds. The one-minute floor is a floor on bucket width, not on the window: when it applies, the series runs past \`timeBounds.end\` and the trailing buckets are empty rather than the window being compressed. A folder path covers its whole subtree. Per-workflow series are capped and \`workflowsTruncated\` reports whether the cap applied; the workspace totals are always computed from every workflow. ${RUN_RETENTION} ${FOLDER_TREE_TOO_LARGE}`,
211211
errors: [...RESOURCE_ERRORS, 'PayloadTooLarge'],
212212
success: { description: 'Bucketed execution statistics for the workspace.' },
213213
}),

apps/sim/lib/logs/application/get-log-stats.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ export const getLogStats = defineAuthorizedWorkspaceUseCase({
4747
)
4848

4949
const bounds = await readLogStatsBounds(where)
50-
const window = resolveLogStatsWindow(bounds, input.segmentCount)
50+
const window = resolveLogStatsWindow(bounds, input.segmentCount, {
51+
requestedStart: input.filters.startDate,
52+
requestedEnd: input.filters.endDate,
53+
})
5154
const rows = await readLogStatsSegments(where, window.startTime.toISOString(), window.segmentMs)
5255
return buildDashboardStats(rows, window, input.segmentCount, {
5356
maxWorkflows: MAX_STATS_WORKFLOWS,

apps/sim/lib/logs/application/log-analytics-use-cases.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,36 @@ describe('getLogStats', () => {
129129
)
130130
})
131131

132+
/**
133+
* The wiring, not the arithmetic: `resolveLogStatsWindow` is exercised for
134+
* real here, so a requested window that never reaches it shows up as both a
135+
* wrong segment origin on the read and a wrong `timeBounds` on the response.
136+
*/
137+
it('spans the requested window rather than the rows that happen to exist', async () => {
138+
const { stats } = await getLogStats.execute({
139+
principal: workspacePrincipal,
140+
input: {
141+
workspaceId: 'workspace-1',
142+
filters: {
143+
startDate: new Date('2026-08-01T00:00:00.000Z'),
144+
endDate: new Date('2026-08-02T00:00:00.000Z'),
145+
},
146+
segmentCount: 2,
147+
},
148+
})
149+
150+
expect(mocks.readSegments).toHaveBeenCalledWith(
151+
expect.anything(),
152+
'2026-08-01T00:00:00.000Z',
153+
12 * 60 * 60 * 1000
154+
)
155+
expect(stats.timeBounds).toEqual({
156+
start: '2026-08-01T00:00:00.000Z',
157+
end: '2026-08-02T00:00:00.000Z',
158+
})
159+
expect(stats.segmentMs).toBe(12 * 60 * 60 * 1000)
160+
})
161+
132162
it('resolves the folder scope only after authorization, and only when asked', async () => {
133163
await getLogStats.execute({
134164
principal: workspacePrincipal,

apps/sim/lib/logs/stats.test.ts

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('resolveLogStatsWindow', () => {
2929
const now = new Date('2026-01-15T12:00:00.000Z')
3030

3131
it('falls back to the trailing 24 hours when nothing ran', () => {
32-
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 24, now)
32+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 24, { now })
3333

3434
expect(resolved.endTime).toEqual(now)
3535
expect(resolved.startTime).toEqual(new Date('2026-01-14T12:00:00.000Z'))
@@ -39,7 +39,7 @@ describe('resolveLogStatsWindow', () => {
3939
const resolved = resolveLogStatsWindow(
4040
{ minTime: '2026-01-15T00:00:00.000Z', maxTime: '2026-01-15T06:00:00.000Z' },
4141
12,
42-
now
42+
{ now }
4343
)
4444

4545
expect(resolved.endTime).toEqual(now)
@@ -50,17 +50,96 @@ describe('resolveLogStatsWindow', () => {
5050
const resolved = resolveLogStatsWindow(
5151
{ minTime: '2026-01-15T12:00:00.000Z', maxTime: '2026-01-15T12:00:01.000Z' },
5252
500,
53-
now
53+
{ now }
5454
)
5555

5656
expect(resolved.segmentMs).toBe(60_000)
5757
})
5858

5959
it('divides by segmentCount without producing a zero-width bucket', () => {
60-
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 1, now)
60+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 1, { now })
6161

6262
expect(resolved.segmentMs).toBe(24 * 60 * 60 * 1000)
6363
})
64+
65+
/**
66+
* The `segmentMs` assertion is the load-bearing half: pinning `endTime`
67+
* alone would still pass for a fix that relabelled `timeBounds` without
68+
* re-deriving the bucket width the series is stamped from.
69+
*/
70+
it('ends the window at the requested end rather than at now', () => {
71+
const resolved = resolveLogStatsWindow(
72+
{ minTime: '2026-01-14T00:00:00.000Z', maxTime: '2026-01-14T06:00:00.000Z' },
73+
12,
74+
{ requestedEnd: new Date('2026-01-14T12:00:00.000Z'), now }
75+
)
76+
77+
expect(resolved.endTime).toEqual(new Date('2026-01-14T12:00:00.000Z'))
78+
expect(resolved.segmentMs).toBe(60 * 60 * 1000)
79+
})
80+
81+
it('starts the window at the requested start rather than at the oldest run', () => {
82+
const resolved = resolveLogStatsWindow(
83+
{ minTime: '2026-01-14T06:00:00.000Z', maxTime: '2026-01-14T09:00:00.000Z' },
84+
12,
85+
{
86+
requestedStart: new Date('2026-01-14T00:00:00.000Z'),
87+
requestedEnd: new Date('2026-01-14T12:00:00.000Z'),
88+
now,
89+
}
90+
)
91+
92+
expect(resolved.startTime).toEqual(new Date('2026-01-14T00:00:00.000Z'))
93+
expect(resolved.segmentMs).toBe(60 * 60 * 1000)
94+
})
95+
96+
it('reports the requested window when nothing ran inside it', () => {
97+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 6, {
98+
requestedStart: new Date('2026-01-01T00:00:00.000Z'),
99+
requestedEnd: new Date('2026-01-07T00:00:00.000Z'),
100+
now,
101+
})
102+
103+
expect(resolved.startTime).toEqual(new Date('2026-01-01T00:00:00.000Z'))
104+
expect(resolved.endTime).toEqual(new Date('2026-01-07T00:00:00.000Z'))
105+
expect(resolved.segmentMs).toBe(24 * 60 * 60 * 1000)
106+
})
107+
108+
/**
109+
* The case neither fallback sentence covers on its own: with no rows and only
110+
* a right edge, the 24-hour window is measured back from the requested end,
111+
* not from the wall clock.
112+
*/
113+
it('measures the empty-result fallback back from a requested end', () => {
114+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 24, {
115+
requestedEnd: new Date('2026-01-10T00:00:00.000Z'),
116+
now,
117+
})
118+
119+
expect(resolved.endTime).toEqual(new Date('2026-01-10T00:00:00.000Z'))
120+
expect(resolved.startTime).toEqual(new Date('2026-01-09T00:00:00.000Z'))
121+
})
122+
123+
/** The dashboard schema has no `startDate <= endDate` refinement, so a crossed pair reaches here. */
124+
it('keeps a crossed requested pair from producing a non-positive bucket width', () => {
125+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 6, {
126+
requestedStart: new Date('2026-01-07T00:00:00.000Z'),
127+
requestedEnd: new Date('2026-01-01T00:00:00.000Z'),
128+
now,
129+
})
130+
131+
expect(resolved.segmentMs).toBe(60_000)
132+
})
133+
134+
it('ignores an unparseable requested bound instead of stamping Invalid Date', () => {
135+
const resolved = resolveLogStatsWindow({ minTime: null, maxTime: null }, 24, {
136+
requestedEnd: new Date('not-a-date'),
137+
now,
138+
})
139+
140+
expect(resolved.endTime).toEqual(now)
141+
expect(resolved.startTime).toEqual(new Date('2026-01-14T12:00:00.000Z'))
142+
})
64143
})
65144

66145
describe('buildDashboardStats', () => {

apps/sim/lib/logs/stats.ts

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,32 +11,64 @@ export interface LogStatsWindow {
1111
segmentMs: number
1212
}
1313

14+
/** Requested window and clock overrides for {@link resolveLogStatsWindow}. */
15+
export interface ResolveLogStatsWindowOptions {
16+
/** Caller-supplied left edge, when the request named one. */
17+
requestedStart?: Date
18+
/** Caller-supplied right edge, when the request named one. */
19+
requestedEnd?: Date
20+
/** Wall clock, injectable so the fallbacks are deterministic under test. */
21+
now?: Date
22+
}
23+
24+
/** An unparseable bound is treated as absent rather than as `Invalid Date`. */
25+
function usableBound(bound: Date | undefined): Date | undefined {
26+
return bound && Number.isFinite(bound.getTime()) ? bound : undefined
27+
}
28+
1429
/**
15-
* The window the segments span, derived from the rows that exist rather than
16-
* from a caller-supplied range.
30+
* The window the segments span.
31+
*
32+
* An edge the caller named wins outright: no run outside it can be counted, so
33+
* deriving the span from anything else stamps trailing buckets the query has
34+
* already excluded and computes `segmentMs` over a width nobody asked for.
1735
*
18-
* A workspace with no runs still has to answer with a window, because
36+
* An omitted edge still falls back to the rows that exist — the oldest matching
37+
* run on the left, and on the right the later of the newest matching run and
38+
* `now`, so a live dashboard's right edge is the present rather than the last
39+
* thing that happened.
40+
*
41+
* A workspace with no matching runs still has to answer with a window, because
1942
* `segmentMs` and every segment timestamp are computed from one — hence the
20-
* trailing-24-hour fallback. The end is pushed to `now` whenever the newest run
21-
* is older than that, so a live dashboard's right edge is the present rather
22-
* than the last thing that happened.
43+
* 24-hour fallback. It is measured back from the right edge, so an empty result
44+
* with no bounds reports the trailing 24 hours, and an empty result with only
45+
* an `endDate` reports the 24 hours preceding that date.
2346
*/
2447
export function resolveLogStatsWindow(
2548
bounds: LogStatsBounds,
2649
segmentCount: number,
27-
now: Date = new Date()
50+
options: ResolveLogStatsWindowOptions = {}
2851
): LogStatsWindow {
52+
const requestedStart = usableBound(options.requestedStart)
53+
const requestedEnd = usableBound(options.requestedEnd)
54+
const now = options.now ?? new Date()
55+
2956
let startTime: Date
3057
let endTime: Date
3158

3259
if (!bounds.minTime || !bounds.maxTime) {
33-
endTime = now
34-
startTime = new Date(now.getTime() - 24 * 60 * 60 * 1000)
60+
endTime = requestedEnd ?? now
61+
startTime = requestedStart ?? new Date(endTime.getTime() - 24 * 60 * 60 * 1000)
3562
} else {
36-
startTime = new Date(bounds.minTime)
37-
endTime = new Date(Math.max(new Date(bounds.maxTime).getTime(), now.getTime()))
63+
startTime = requestedStart ?? new Date(bounds.minTime)
64+
endTime = requestedEnd ?? new Date(Math.max(new Date(bounds.maxTime).getTime(), now.getTime()))
3865
}
3966

67+
/**
68+
* A crossed pair reaches here from the first-party dashboard, whose query
69+
* schema carries no `startDate <= endDate` refinement, so the floor is what
70+
* keeps `segmentMs` positive instead of zero or negative.
71+
*/
4072
const totalMs = Math.max(1, endTime.getTime() - startTime.getTime())
4173
return {
4274
startTime,

apps/sim/lib/workflows/application/replace-workflow-state.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ const mocks = vi.hoisted(() => ({
1111
resolvePermission: vi.fn(),
1212
notify: vi.fn(),
1313
replace: vi.fn(),
14+
prepare: vi.fn(),
15+
collectGraphIds: vi.fn(),
16+
assertIdsUnclaimed: vi.fn(),
1417
validate: vi.fn(),
1518
needsRedeployment: vi.fn(),
1619
}))
@@ -35,8 +38,13 @@ vi.mock('@/lib/workflows/application/context', () => ({
3538
resolveActiveWorkflowApplicationContext: mocks.resolveContext,
3639
}))
3740
vi.mock('@/lib/realtime/notify', () => ({ notifyWorkflowUpdated: mocks.notify }))
41+
vi.mock('@/lib/workflows/persistence/prepare-state', () => ({
42+
prepareWorkflowStateForPersistence: mocks.prepare,
43+
}))
3844
vi.mock('@/lib/workflows/persistence/replace-normalized-state', () => ({
3945
replaceWorkflowNormalizedState: mocks.replace,
46+
collectWorkflowGraphIds: mocks.collectGraphIds,
47+
assertWorkflowGraphIdsUnclaimed: mocks.assertIdsUnclaimed,
4048
}))
4149
vi.mock('@/lib/workflows/sanitization/validation', () => ({
4250
validateWorkflowState: mocks.validate,
@@ -88,6 +96,12 @@ describe('replaceWorkflowState', () => {
8896
state: { blocks: { 'block-1': BLOCK }, edges: [], loops: {}, parallels: {} },
8997
})
9098
mocks.needsRedeployment.mockResolvedValue(true)
99+
mocks.prepare.mockReturnValue({
100+
state: { blocks: { 'block-1': BLOCK }, edges: [], loops: {}, parallels: {} },
101+
warnings: [],
102+
})
103+
mocks.collectGraphIds.mockReturnValue({ blockIds: ['block-1'], edgeIds: [], subflowIds: [] })
104+
mocks.assertIdsUnclaimed.mockResolvedValue(undefined)
91105
})
92106

93107
/**
@@ -333,6 +347,77 @@ describe('replaceWorkflowState', () => {
333347
expect(dry.edgesCount).toBe(committed.edgesCount)
334348
})
335349

350+
/**
351+
* The preview promised in {@link ReplaceWorkflowStateInput.dryRun} is
352+
* byte-identical to the committed write of the same body, and preparation
353+
* is where a dropped edge or a stripped inline secret is noted. Reporting
354+
* only the validation half made the dry run quietly less informative than
355+
* the write it previews.
356+
*/
357+
it('merges the preparation warnings a committed write would report', async () => {
358+
mocks.validate.mockReturnValue({
359+
valid: true,
360+
errors: [],
361+
warnings: ['Dropped block "block-2"'],
362+
})
363+
mocks.prepare.mockReturnValue({
364+
state: { blocks: { 'block-1': BLOCK }, edges: [], loops: {}, parallels: {} },
365+
warnings: ['Dropped edge "edge-9": target block does not exist'],
366+
})
367+
mocks.replace.mockResolvedValue({
368+
warnings: ['Dropped edge "edge-9": target block does not exist'],
369+
state: { blocks: { 'block-1': BLOCK }, edges: [], loops: {}, parallels: {} },
370+
})
371+
372+
const dry = await replaceWorkflowState.execute({
373+
principal: sessionPrincipal,
374+
input: { ...input, dryRun: true },
375+
})
376+
const committed = await replaceWorkflowState.execute({ principal: sessionPrincipal, input })
377+
378+
expect(dry.warnings).toEqual([
379+
'Dropped block "block-2"',
380+
'Dropped edge "edge-9": target block does not exist',
381+
])
382+
expect(dry.warnings).toEqual(committed.warnings)
383+
})
384+
385+
/**
386+
* A dry run that reports clean for a body that cannot commit is worse than
387+
* the fault it hides. It checks the ids the write would actually insert —
388+
* the prepared graph's, not the caller's body's.
389+
*/
390+
it('refuses a graph whose ids another workflow already owns', async () => {
391+
mocks.assertIdsUnclaimed.mockRejectedValueOnce(
392+
new OrchestrationError('conflict', 'Block ids already used by another workflow: block-1')
393+
)
394+
395+
await expect(
396+
replaceWorkflowState.execute({
397+
principal: sessionPrincipal,
398+
input: { ...input, dryRun: true },
399+
})
400+
).rejects.toMatchObject({ code: 'conflict' })
401+
expect(mocks.replace).not.toHaveBeenCalled()
402+
})
403+
404+
it('checks the ids the prepared graph would insert, not the ids sent', async () => {
405+
const prepared = { blocks: { 'block-1': BLOCK }, edges: [], loops: {}, parallels: {} }
406+
mocks.prepare.mockReturnValue({ state: prepared, warnings: [] })
407+
408+
await replaceWorkflowState.execute({
409+
principal: sessionPrincipal,
410+
input: { ...input, dryRun: true },
411+
})
412+
413+
expect(mocks.collectGraphIds).toHaveBeenCalledWith(prepared)
414+
expect(mocks.assertIdsUnclaimed).toHaveBeenCalledWith(expect.anything(), 'workflow-1', {
415+
blockIds: ['block-1'],
416+
edgeIds: [],
417+
subflowIds: [],
418+
})
419+
})
420+
336421
/** A locked workflow refuses the preview too, or the preview would lie. */
337422
it('refuses when the workflow cannot be mutated', async () => {
338423
workflowAuthzMockFns.mockAssertWorkflowMutable.mockRejectedValueOnce(

0 commit comments

Comments
 (0)