fix(migrations): keep a ServiceNow write body off the read projection

Review findings from the first round.

- A legacy ServiceNow block can hold a Create/Update Record JSON body under
  `fields` while its stored operation is Read Records: the id served both value
  spaces before the rename, and a subblock value is not cleared when the
  operation changes. The scoped migration moved that body onto `readFields`,
  where it would reach the wire as sysparm_fields. Migration entries can now
  carry a `whenValue` predicate for the case where the stored operation alone
  cannot separate two value spaces, and the ServiceNow entry uses it to move
  only a plausible comma-separated projection.
- Type the fork copy test harness instead of using `any`, without weakening it:
  every predicate shape it does not model still throws rather than matching.
- Correct the dependent-omission comments. Omitting a parent-invalidated field
  preserves the target's stored value on Save and across an undo, where the
  parent nets out unchanged; on a Sync the written state is source-derived, so
  what it prevents there is an explicit blank reaching the fields the remap's
  clearing pass does not cover, nested tool params in particular.

Okta's migration scope is left as-is: `okta_remove_user_from_app` and the
sendEmail split shipped in the same release, so no saved block can hold legacy
state for it, and widening the scope would promote an activation-era value onto
the deactivation switch. Tests document the boundary.

Author: waleedlatif1

apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts

@@ -13,8 +13,17 @@ function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentRe
  * Marker stored for a descendant whose value an in-session parent re-pick invalidated, kept
  * distinct from the user's own empty pick. It reads as blank everywhere it is consumed - the
  * selector, the in-block chain context, and the sync gate - but is never submitted: the user
- * has not chosen a replacement, so the target keeps its stored value instead of being blanked.
- * A `''` the user picked themselves IS submitted and does clear the target.
+ * has not chosen a replacement, so no override is written for it. A `''` the user picked
+ * themselves IS submitted and does clear the target.
+ *
+ * What the omission buys differs by path. On Save nothing rewrites the target draft, so its
+ * stored value survives outright - including across an undo (re-pick away, then back), where
+ * the parent nets out unchanged so `clearDependentsOnRemap` never fires and an explicit `''`
+ * would land on a value nobody touched. On Sync the written state is source-derived and
+ * `clearDependentsOnRemap` already blanks every top-level dependent of a remapped parent, so
+ * omission preserves nothing there; what it prevents is an explicit `''` reaching the fields
+ * that pass does not cover - nested `tools[i].param` values in particular, which
+ * `applyNestedToolOverrides` would otherwise blank.
  *
  * The escaped NUL prefix keeps it disjoint from every real selector value (ids, names, label
  * paths) - no selector can produce one, so it can never collide with a genuine pick.

apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts

@@ -688,9 +688,11 @@ export function useForkSync(params: {
   // or the source reference; promote translates a source document id to its copied counterpart
   // at write time). The server persists this verbatim as the stored mapping; fields whose
   // parent is unresolved are omitted (they can't be configured), as are fields an in-block
-  // parent re-pick invalidated and the user never re-picked - writing those blank would destroy
-  // a stored target value nobody chose to clear. This is the whole "what's in the mapping goes
-  // in" contract, shared by Save and Sync so the two persist identically.
+  // parent re-pick invalidated and the user never re-picked - submitting those blank writes an
+  // explicit `''` override into the target draft (see `applyDependentOverrides`), which on the
+  // paths `clearDependentsOnRemap` does not cover would clear a value nobody chose to clear.
+  // `DEPENDENT_CLEARED_BY_PARENT` documents which paths those are. This is the whole "what's in
+  // the mapping goes in" contract, shared by Save and Sync so the two persist identically.
   const buildDependentValues = () =>
     dependentReconfigs.flatMap((field) => {
       const parent = entryForDependent(field)

apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts

@@ -4,12 +4,25 @@
 import { folder as folderTable } from '@sim/db/schema'
 import {
   dbChainMockFns,
+  type MockCondition,
   resetDbChainMock,
   storageServiceMock,
   storageServiceMockFns,
 } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
+/** The `workspace_files` columns {@link fileRows} enforces its unique indexes on. */
+interface WorkspaceFileRow {
+  id: string
+  key: string
+  workspaceId: string | null
+  folderId?: string | null
+  context: string
+  originalName: string
+  deletedAt: Date | null
+  [column: string]: unknown
+}
+
 /**
  * `fileRows` is the shared stand-in for the `workspace_files` table: the mocked name
  * allocator reads it exactly as the real one queries the DB, and the insert simulation in
@@ -23,17 +36,7 @@ const {
   mockIncrementStorageUsageInTx,
   mockResolveStorageBillingContext,
 } = vi.hoisted(() => {
-  interface FileRow {
-    id: string
-    key: string
-    workspaceId: string | null
-    folderId?: string | null
-    context: string
-    originalName: string
-    deletedAt: Date | null
-    [column: string]: unknown
-  }
-  const fileRows: FileRow[] = []
+  const fileRows: WorkspaceFileRow[] = []
   const withCopySuffix = (name: string, n: number) => {
     const lastDot = name.lastIndexOf('.')
     return lastDot > 0 && lastDot < name.length - 1
@@ -247,39 +250,76 @@ describe('executeForkFileBlobCopies storage accounting', () => {
   })
 })
 
+/** Rejects a predicate the harness does not model, rather than letting it match everything. */
+function unsupportedPredicate(detail: string): never {
+  throw new Error(`Unsupported predicate in test harness: ${detail}`)
+}
+
+/** The nested clauses of an `and`/`or` node, or a throw when the node carries none. */
+function predicateClauses(node: MockCondition): unknown[] {
+  if (!Array.isArray(node.conditions))
+    unsupportedPredicate(`${String(node.type)} without conditions`)
+  return node.conditions
+}
+
+/**
+ * The row key a predicate node references. The mocked schema tables are column-name maps, so a
+ * column reference is the column name itself; anything else is a shape this harness cannot read.
+ */
+function predicateColumn(node: MockCondition, field: 'left' | 'column'): string {
+  const column = node[field]
+  if (typeof column !== 'string')
+    unsupportedPredicate(`${String(node.type)} with a non-column ${field}`)
+  return column
+}
+
 /**
  * Evaluate a mocked drizzle predicate against a row. Real predicate reading, so a chain that
  * ignores its `where` clause cannot pass these tests by echoing a fixture back.
  */
-function matchesPredicate(row: Record<string, unknown>, predicate: any): boolean {
+function matchesPredicate(row: Record<string, unknown>, predicate: unknown): boolean {
   if (!predicate) return true
-  switch (predicate.type) {
+  if (typeof predicate !== 'object') unsupportedPredicate(typeof predicate)
+  const node = predicate as MockCondition
+  switch (node.type) {
     case 'and':
-      return predicate.conditions.every((clause: unknown) => matchesPredicate(row, clause))
+      return predicateClauses(node).every((clause) => matchesPredicate(row, clause))
     case 'or':
-      return predicate.conditions.some((clause: unknown) => matchesPredicate(row, clause))
+      return predicateClauses(node).some((clause) => matchesPredicate(row, clause))
     case 'eq':
-      return row[predicate.left] === predicate.right
-    case 'isNull':
-      return row[predicate.column] === null || row[predicate.column] === undefined
-    case 'inArray':
-      return predicate.values.includes(row[predicate.column])
+      return row[predicateColumn(node, 'left')] === node.right
+    case 'isNull': {
+      const value = row[predicateColumn(node, 'column')]
+      return value === null || value === undefined
+    }
+    case 'inArray': {
+      if (!Array.isArray(node.values)) unsupportedPredicate('inArray without values')
+      return node.values.includes(row[predicateColumn(node, 'column')])
+    }
     default:
-      throw new Error(`Unsupported predicate in test harness: ${predicate?.type}`)
+      return unsupportedPredicate(String(node.type))
   }
 }
 
 /** Awaitable stand-in for a drizzle select result, supporting `.limit`/`.for`/`.orderBy`. */
-function selectResult(rows: Record<string, unknown>[]): any {
-  const builder: any = {
-    then: (onFulfilled?: any, onRejected?: any) =>
-      Promise.resolve(rows).then(onFulfilled, onRejected),
-    catch: (onRejected?: any) => Promise.resolve(rows).catch(onRejected),
-    finally: (onFinally?: any) => Promise.resolve(rows).finally(onFinally),
+interface MockSelectResult extends PromiseLike<WorkspaceFileRow[]> {
+  catch: Promise<WorkspaceFileRow[]>['catch']
+  finally: Promise<WorkspaceFileRow[]>['finally']
+  limit: (count: number) => MockSelectResult
+  for: () => MockSelectResult
+  orderBy: () => MockSelectResult
+}
+
+function selectResult(rows: WorkspaceFileRow[]): MockSelectResult {
+  const settled = Promise.resolve(rows)
+  const builder: MockSelectResult = {
+    then: (onFulfilled, onRejected) => settled.then(onFulfilled, onRejected),
+    catch: (onRejected) => settled.catch(onRejected),
+    finally: (onFinally) => settled.finally(onFinally),
+    limit: (count: number) => selectResult(rows.slice(0, count)),
+    for: () => builder,
+    orderBy: () => builder,
   }
-  builder.limit = (count: number) => selectResult(rows.slice(0, count))
-  builder.for = () => builder
-  builder.orderBy = () => builder
   return builder
 }
 
@@ -294,7 +334,7 @@ function installFileTableSimulation(): void {
   dbChainMockFns.where.mockImplementation((predicate: unknown) =>
     selectResult(fileRows.filter((row) => matchesPredicate(row, predicate)))
   )
-  dbChainMockFns.values.mockImplementation((row: any) => {
+  dbChainMockFns.values.mockImplementation((row: WorkspaceFileRow) => {
     const attemptInsert = (conflictTarget: unknown) => {
       const pkConflict = fileRows.some((existing) => existing.id === row.id)
       const activeConflict = fileRows.some(

apps/sim/lib/workflows/migrations/subblock-migrations.test.ts

@@ -529,6 +529,138 @@ describe('migrateSubblockIds', () => {
     })
   })
 
+  describe('servicenow block', () => {
+    it('moves a legacy Read Records projection onto readFields', () => {
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'servicenow',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'servicenow_read_record' },
+            fields: {
+              id: 'fields',
+              type: 'short-input',
+              value: 'number,short_description,priority',
+            },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(true)
+      expect(blocks.b1.subBlocks.readFields.value).toBe('number,short_description,priority')
+      expect(blocks.b1.subBlocks.fields).toBeUndefined()
+    })
+
+    /**
+     * The shipped block shared `fields` between the Create/Update Record JSON
+     * body and the Read Records projection, and a subblock value survives an
+     * operation switch. So `operation: servicenow_read_record` holding a JSON
+     * body under `fields` is a reachable saved state, and promoting that body
+     * onto `readFields` would send it as `sysparm_fields`.
+     */
+    it('leaves a Create Record JSON body under fields when the operation was switched to Read Records', () => {
+      const body = '{\n  "short_description": "Issue description",\n  "priority": "1"\n}'
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'servicenow',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'servicenow_read_record' },
+            fields: { id: 'fields', type: 'code', value: body },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(false)
+      expect(blocks.b1.subBlocks.readFields).toBeUndefined()
+      expect(blocks.b1.subBlocks.fields.value).toBe(body)
+    })
+
+    it('leaves a JSON array value under fields as well', () => {
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'servicenow',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'servicenow_read_record' },
+            fields: { id: 'fields', type: 'code', value: '["short_description"]' },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(false)
+      expect(blocks.b1.subBlocks.readFields).toBeUndefined()
+      expect(blocks.b1.subBlocks.fields.value).toBe('["short_description"]')
+    })
+
+    it('leaves the JSON body alone on create', () => {
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'servicenow',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'servicenow_create_record' },
+            fields: { id: 'fields', type: 'code', value: '{"short_description":"x"}' },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(false)
+      expect(blocks.b1.subBlocks.readFields).toBeUndefined()
+      expect(blocks.b1.subBlocks.fields.value).toBe('{"short_description":"x"}')
+    })
+  })
+
+  /**
+   * `okta_remove_user_from_app` reached the block in #6741 (`d45dad7e8b`),
+   * whose only release tag is v0.8.3 — the same release that split `sendEmail`
+   * into `sendDeactivationEmail`. In v0.8.2 the operation does not exist and
+   * `sendEmail` covers only activate/deactivate/reset/delete, so no saved state
+   * can hold a remove-from-app preference under `sendEmail`, and widening the
+   * scope would only let an activation-era value be promoted.
+   */
+  describe('okta block', () => {
+    it('renames the deactivation half of the shared send-email switch', () => {
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'okta',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'okta_deactivate_user' },
+            sendEmail: { id: 'sendEmail', type: 'switch', value: 'true' },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(true)
+      expect(blocks.b1.subBlocks.sendDeactivationEmail.value).toBe('true')
+      expect(blocks.b1.subBlocks.sendEmail).toBeUndefined()
+    })
+
+    it('leaves the activation half on sendEmail', () => {
+      const input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'okta',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'okta_activate_user' },
+            sendEmail: { id: 'sendEmail', type: 'switch', value: 'false' },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(false)
+      expect(blocks.b1.subBlocks.sendEmail.value).toBe('false')
+      expect(blocks.b1.subBlocks.sendDeactivationEmail).toBeUndefined()
+    })
+  })
+
   it('should handle blocks with empty subBlocks', () => {
     const input: Record<string, BlockState> = {
       b1: makeBlock({ type: 'knowledge', subBlocks: {} }),