Skip to content

Commit 8616b6b

Browse files
committed
fix(v2): make the NUL path scan linear, and force a write surface to choose
Two findings from a simplify pass, both in code this branch added. findNulBytePath copied `[...path, key]` per child, which is O(nodes x depth). A caller controls that depth directly: v2 row cell values are `z.unknown()`, so nesting passes Zod untouched and reaches the scan. Measured on Node 22 -- JSON.parse accepts a 200KB body nested 100k deep in 9.8ms, and the scan then blocked the event loop for 27.7s. Frames now carry a parent link and the path is materialized once, for the node actually reported: 27.7s -> 5ms, with byte-identical paths across nested arrays, records, NUL keys and clean input. The always-run first pass drops Object.entries for Object.keys, which halves its cost on large bodies by not allocating a pair array per object. `strictWrite` was optional with the lenient default, so a v2 write route added tomorrow would silently inherit first-party behavior -- unknown column dropped under a 201, uncoercible cell stored as null -- defended by nothing but five copies of a literal. It is now required on the five write-shaped inputs, so omission is a compile error. The type-checker named every caller: the five v2 routes already passed true, and the three Copilot sites now say false explicitly, which is the behavior they already had.
1 parent 9f693e1 commit 8616b6b

3 files changed

Lines changed: 51 additions & 16 deletions

File tree

apps/sim/lib/api/server/nul-bytes.ts

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,42 +21,63 @@ function containsNulByte(root: unknown): boolean {
2121
continue
2222
}
2323
if (isPlainRecord(value)) {
24-
for (const [key, entry] of Object.entries(value)) {
24+
for (const key of Object.keys(value)) {
2525
if (containsNulCharacter(key)) return true
26-
stack.push(entry)
26+
stack.push(value[key])
2727
}
2828
}
2929
}
3030
return false
3131
}
3232

33+
/** A visited node, linked to its parent so a path is only ever built on a hit. */
34+
interface NulScanFrame {
35+
value: unknown
36+
key: PropertyKey | null
37+
parent: NulScanFrame | null
38+
}
39+
40+
/** Walks parent links back to the root. Runs once, only for the offending node. */
41+
function framePath(frame: NulScanFrame): PropertyKey[] {
42+
const path: PropertyKey[] = []
43+
for (let node: NulScanFrame | null = frame; node?.parent; node = node.parent) {
44+
if (node.key !== null) path.push(node.key)
45+
}
46+
return path.reverse()
47+
}
48+
3349
/**
34-
* Second pass, run only once a NUL is known to be present, so the common case
35-
* never pays for path bookkeeping. Returns the path of the first offending
36-
* string, matching the shape Zod reports for a failed field.
50+
* Second pass, run only once a NUL is known to be present. Returns the path of
51+
* the first offending string, matching the shape Zod reports for a failed field.
52+
*
53+
* Frames carry a parent link rather than a copied path. Copying `[...path, key]`
54+
* per child costs O(nodes x depth), which a caller controls directly: v2 row
55+
* cell values are `z.unknown()`, so a 200KB body of nested arrays reaches this
56+
* scan at depth 100k and blocked the event loop for ~28s. Parent links make it
57+
* linear, and the path is materialized once for the node actually reported.
3758
*/
3859
function findNulBytePath(root: unknown): PropertyKey[] {
39-
const stack: { value: unknown; path: PropertyKey[] }[] = [{ value: root, path: [] }]
60+
const stack: NulScanFrame[] = [{ value: root, key: null, parent: null }]
4061
while (stack.length > 0) {
4162
const frame = stack.pop()
4263
if (!frame) break
43-
const { value, path } = frame
64+
const { value } = frame
4465
if (typeof value === 'string') {
45-
if (containsNulCharacter(value)) return path
66+
if (containsNulCharacter(value)) return framePath(frame)
4667
continue
4768
}
4869
if (Array.isArray(value)) {
4970
for (let index = value.length - 1; index >= 0; index -= 1) {
50-
stack.push({ value: value[index], path: [...path, index] })
71+
stack.push({ value: value[index], key: index, parent: frame })
5172
}
5273
continue
5374
}
5475
if (isPlainRecord(value)) {
55-
const entries = Object.entries(value)
56-
for (let index = entries.length - 1; index >= 0; index -= 1) {
57-
const [key, entry] = entries[index]
58-
if (containsNulCharacter(key)) return [...path, key]
59-
stack.push({ value: entry, path: [...path, key] })
76+
const keys = Object.keys(value)
77+
for (let index = keys.length - 1; index >= 0; index -= 1) {
78+
const key = keys[index]
79+
if (containsNulCharacter(key)) return [...framePath(frame), key]
80+
stack.push({ value: value[key], key, parent: frame })
6081
}
6182
}
6283
}

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
285285
kind: 'single',
286286
tableId: args.tableId,
287287
assertedWorkspaceId: workspaceId,
288+
strictWrite: false,
288289
data: args.data,
289290
position: args.position as number | undefined,
290291
secretProvenance: createExactEmptyTableRowSecretProvenance(args.data),
@@ -327,6 +328,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
327328
kind: 'batch',
328329
tableId: args.tableId,
329330
assertedWorkspaceId: workspaceId,
331+
strictWrite: false,
330332
rows: sourceRows,
331333
secretProvenance: sourceRows.map(createExactEmptyTableRowSecretProvenance),
332334
},
@@ -473,6 +475,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
473475
{
474476
tableId: args.tableId,
475477
assertedWorkspaceId: workspaceId,
478+
strictWrite: false,
476479
rowId: args.rowId,
477480
data: args.data,
478481
secretProvenance: createExactEmptyTableRowSecretProvenance(args.data),

apps/sim/lib/table/application/rows.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,10 @@ interface TableScopedInput {
9292
* the Copilot table tools, and the executor's Table block all drop the
9393
* unknown key and blank the uncoercible cell. Read-only use cases ignore it.
9494
*/
95-
strictWrite?: boolean
9695
}
9796

9897
/** The write policy `strictWrite` selects, for the row-service primitives. */
99-
function rowWriteOptions(input: TableScopedInput): RowWriteOptions {
98+
function rowWriteOptions(input: { strictWrite: boolean }): RowWriteOptions {
10099
return input.strictWrite ? { uncoercibleValues: 'reject' } : {}
101100
}
102101

@@ -419,6 +418,8 @@ export const readTableRow = defineAuthorizedTableUseCase({
419418
})
420419

421420
interface CreateSingleTableRowInput extends TableScopedInput {
421+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
422+
strictWrite: boolean
422423
kind: 'single'
423424
data: RowData
424425
position?: number
@@ -428,6 +429,8 @@ interface CreateSingleTableRowInput extends TableScopedInput {
428429
}
429430

430431
interface CreateBatchTableRowsInput extends TableScopedInput {
432+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
433+
strictWrite: boolean
431434
kind: 'batch'
432435
rows: RowData[]
433436
orderKeys?: string[]
@@ -527,6 +530,8 @@ export const createTableRows = defineAuthorizedTableUseCase({
527530
const MAX_REPLACE_TABLE_ROWS = 10_000
528531

529532
export interface ReplaceTableRowsInput extends TableScopedInput {
533+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
534+
strictWrite: boolean
530535
rows: RowData[]
531536
secretProvenance?: Array<TableRowSecretProvenanceWrite | undefined>
532537
}
@@ -735,6 +740,8 @@ export const replaceProjectedWireRows = defineAuthorizedTableUseCase({
735740
})
736741

737742
export interface UpdateTableRowInput extends TableScopedInput {
743+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
744+
strictWrite: boolean
738745
rowId: string
739746
data: RowData
740747
secretProvenance?: TableRowSecretProvenanceWrite
@@ -784,6 +791,8 @@ export const updateTableRow = defineAuthorizedTableUseCase({
784791
})
785792

786793
export interface UpdateTableRowsInput extends TableScopedInput {
794+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
795+
strictWrite: boolean
787796
filter: TablePredicate
788797
data: RowData
789798
limit?: number
@@ -907,6 +916,8 @@ export const deleteTableRows = defineAuthorizedTableUseCase({
907916
})
908917

909918
export interface UpsertTableRowInput extends TableScopedInput {
919+
/** See {@link rowWriteOptions}. Required so a new write surface must choose. */
920+
strictWrite: boolean
910921
data: RowData
911922
conflictTarget?: string
912923
secretProvenance?: TableRowSecretProvenanceWrite

0 commit comments

Comments
 (0)