fix(cloudflare): stop per-operation subblock defaults colliding on a shared id

Subblock initial values are seeded into block state keyed by subblock id —
both stores/workflows/utils.ts and lib/workflows/defaults.ts assign
`subBlocks[subBlock.id]` in a plain forEach — so two controls sharing an id
leave one stored value and the last definition in file order wins. Four ids
were duplicated with differing defaults:

- `type` was defined four times. The Access "Application Type" control is
  last, so every new block seeded `type = 'self_hosted'` and the three DNS
  record controls inherited it — Create DNS Record sent a Zero Trust
  application type as its record type. The subblock added on this branch
  broke a default on tools that shipped long before it.
- `status` was defined three times. The empty tunnel filter is last, so
  List Certificates lost its `all` default.
- `proxied` was defined three times. An empty filter is last, so Create DNS
  Record lost its explicit `false`.
- `action` was defined twice. The rate limiting dropdown is last, so the
  ruleset-rule action input was seeded `block`, quietly making "block live
  traffic" the default for a WAF custom rule the user never configured.

Give the colliding controls their own ids and map them back to the tool
params per operation, ahead of the coercions that read them, so each
operation keeps its own default. The other 17 duplicated ids agree on their
value and are left shared.

Adds tests covering each separated default plus a sweep asserting no id
carries two different seeded values, so a future duplicate goes red.

Author: waleedlatif1

apps/sim/blocks/blocks/cloudflare.ts

@@ -36,7 +36,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
         ],
         create_dns_record: [
           { text: 'Add DNS record', field: 'name', core: true },
-          { text: 'of type', field: 'type' },
+          { text: 'of type', field: 'recordType' },
           { text: ', pointing at', field: 'content' },
         ],
         update_dns_record: [
@@ -50,7 +50,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
         ],
         list_certificates: [
           { text: 'List certificate packs for zone', field: 'zoneId', core: true },
-          { text: ', with status', field: 'status' },
+          { text: ', with status', field: 'certificateStatus' },
         ],
         get_zone_settings: [{ text: 'Read all settings of zone', field: 'zoneId', core: true }],
         update_zone_setting: [
@@ -543,7 +543,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
       condition: { field: 'operation', value: 'create_dns_record' },
     },
     {
-      id: 'type',
+      id: 'recordType',
       title: 'Record Type',
       type: 'dropdown',
       options: [
@@ -583,7 +583,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
       mode: 'advanced',
     },
     {
-      id: 'proxied',
+      id: 'recordProxied',
       title: 'Proxied',
       type: 'dropdown',
       options: [
@@ -744,7 +744,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
       condition: { field: 'operation', value: 'list_certificates' },
     },
     {
-      id: 'status',
+      id: 'certificateStatus',
       title: 'Status Filter',
       type: 'dropdown',
       options: [
@@ -1228,7 +1228,7 @@ Return ONLY the comma-separated URLs - no explanations, no extra text.`,
       },
     },
     {
-      id: 'action',
+      id: 'rateLimitAction',
       title: 'Action',
       type: 'dropdown',
       options: [
@@ -1529,7 +1529,7 @@ Return ONLY the comma-separated list - no explanations, no extra text.`,
       },
     },
     {
-      id: 'type',
+      id: 'appType',
       title: 'Application Type',
       type: 'dropdown',
       options: [
@@ -2241,7 +2241,36 @@ Return ONLY the JSON array - no explanations, no markdown fences.`,
     config: {
       tool: (params) => `cloudflare_${params.operation}`,
       params: (params) => {
-        const result = { ...params }
+        const { recordType, recordProxied, certificateStatus, appType, rateLimitAction, ...rest } =
+          params
+        const result: Record<string, unknown> = rest
+
+        /**
+         * SubBlock defaults are seeded into block state keyed by subBlock id, so
+         * two controls sharing an id leave one stored value and the last
+         * definition silently wins. Controls that map to the same tool param but
+         * need their own default therefore get their own id, and are routed back
+         * here — before any coercion below reads them.
+         */
+        if (result.operation === 'create_dns_record') {
+          result.type = recordType
+          result.proxied = recordProxied
+        }
+        if (result.operation === 'list_certificates') {
+          result.status = certificateStatus
+        }
+        if (
+          result.operation === 'create_access_application' ||
+          result.operation === 'update_access_application'
+        ) {
+          result.type = appType
+        }
+        if (
+          result.operation === 'create_rate_limit_rule' ||
+          result.operation === 'update_rate_limit_rule'
+        ) {
+          result.action = rateLimitAction
+        }
 
         if (result.ttl) result.ttl = Number(result.ttl)
         if (result.priority) result.priority = Number(result.priority)
@@ -2334,6 +2363,17 @@ Return ONLY the JSON array - no explanations, no markdown fences.`,
     recordId: { type: 'string', description: 'DNS record ID' },
     name: { type: 'string', description: 'Domain or record name' },
     type: { type: 'string', description: 'DNS record type' },
+    recordType: { type: 'string', description: 'DNS record type for record creation' },
+    recordProxied: {
+      type: 'string',
+      description: 'Whether the created DNS record is proxied through Cloudflare',
+    },
+    certificateStatus: { type: 'string', description: 'Certificate pack status filter' },
+    appType: { type: 'string', description: 'Access application type' },
+    rateLimitAction: {
+      type: 'string',
+      description: 'Action applied once a rate limit is exceeded',
+    },
     content: { type: 'string', description: 'DNS record content' },
     ttl: { type: 'number', description: 'Time to live in seconds' },
     proxied: { type: 'boolean', description: 'Whether Cloudflare proxy is enabled' },

apps/sim/tools/cloudflare/cloudflare.test.ts

@@ -0,0 +1,145 @@
+/**
+ * @vitest-environment node
+ *
+ * Guards the per-operation subBlock defaults in the Cloudflare block.
+ *
+ * SubBlock initial values are seeded into block state keyed by subBlock id
+ * (`stores/workflows/utils.ts` and `lib/workflows/defaults.ts` both assign
+ * `subBlocks[subBlock.id] = ...` in a plain forEach), so two controls sharing an
+ * id leave a single stored value and the LAST definition in file order wins.
+ * These tests assert the seeded default reaching each tool for operations whose
+ * control is deliberately not last, so re-introducing a collision goes red.
+ */
+import { describe, expect, it } from 'vitest'
+import { CloudflareBlock } from '@/blocks/blocks/cloudflare'
+
+const apiKey = 'cf-token'
+
+const mapParams = CloudflareBlock.tools.config?.params
+
+/**
+ * Reproduces what the stores seed into block state: every subBlock default,
+ * keyed by id, with later definitions overwriting earlier ones.
+ */
+function seededDefaults(): Record<string, unknown> {
+  const seeded: Record<string, unknown> = {}
+  for (const subBlock of CloudflareBlock.subBlocks) {
+    if (typeof subBlock.value === 'function') {
+      seeded[subBlock.id] = (subBlock.value as (p: Record<string, never>) => unknown)({})
+    }
+  }
+  return seeded
+}
+
+function mapFor(operation: string, extra: Record<string, unknown> = {}) {
+  return mapParams?.({
+    ...seededDefaults(),
+    apiKey,
+    operation,
+    ...extra,
+  } as never) as Record<string, unknown>
+}
+
+describe('subBlock ids that share a tool param keep their own default', () => {
+  it('does not leak the Access application type onto DNS record creation', () => {
+    // The Access "Application Type" control is defined after every DNS type
+    // control, so a shared id would seed create_dns_record with self_hosted.
+    const mapped = mapFor('create_dns_record', {
+      zoneId: 'zone1',
+      name: 'www.example.com',
+      content: '203.0.113.10',
+    })
+
+    expect(mapped.type).toBe('A')
+    expect(mapped.type).not.toBe('self_hosted')
+  })
+
+  it('keeps the Access application type on the Access operations', () => {
+    expect(mapFor('create_access_application', { accountId: 'acct1' }).type).toBe('self_hosted')
+    expect(mapFor('update_access_application', { accountId: 'acct1', appId: 'app1' }).type).toBe(
+      'self_hosted'
+    )
+  })
+
+  it('leaves the DNS record type unset on the filter operations', () => {
+    expect(mapFor('list_dns_records', { zoneId: 'zone1' }).type).toBeUndefined()
+    expect(mapFor('update_dns_record', { zoneId: 'zone1', recordId: 'rec1' }).type).toBeUndefined()
+  })
+
+  it('preserves the certificate status default past the later status filters', () => {
+    // list_tunnels defines the last `status` control and defaults it to empty.
+    expect(mapFor('list_certificates', { zoneId: 'zone1' }).status).toBe('all')
+    expect(mapFor('list_zones').status).toBeUndefined()
+    expect(mapFor('list_tunnels', { accountId: 'acct1' }).status).toBeUndefined()
+  })
+
+  it('preserves the created-record proxied default past the later proxied filters', () => {
+    const mapped = mapFor('create_dns_record', {
+      zoneId: 'zone1',
+      name: 'www.example.com',
+      content: '203.0.113.10',
+    })
+
+    expect(mapped.proxied).toBe(false)
+    expect(mapFor('list_dns_records', { zoneId: 'zone1' }).proxied).toBeUndefined()
+  })
+
+  it('does not seed a block action onto a WAF custom rule', () => {
+    // The rate limiting action dropdown defaults to block and is defined after
+    // the ruleset-rule action input; sharing an id would make every new WAF
+    // custom rule silently default to blocking traffic.
+    const mapped = mapFor('create_ruleset_rule', {
+      zoneId: 'zone1',
+      rulesetId: 'rs1',
+      expression: 'true',
+    })
+
+    expect(mapped.action).toBeUndefined()
+  })
+
+  it('keeps the block default on the rate limiting operations', () => {
+    expect(mapFor('create_rate_limit_rule', { zoneId: 'zone1', rulesetId: 'rs1' }).action).toBe(
+      'block'
+    )
+    expect(
+      mapFor('update_rate_limit_rule', { zoneId: 'zone1', rulesetId: 'rs1', ruleId: 'r1' }).action
+    ).toBe('block')
+  })
+
+  it('strips the aliased control ids so they never reach a tool as params', () => {
+    const mapped = mapFor('create_dns_record', { zoneId: 'zone1' })
+
+    for (const alias of [
+      'recordType',
+      'recordProxied',
+      'certificateStatus',
+      'appType',
+      'rateLimitAction',
+    ]) {
+      expect(mapped).not.toHaveProperty(alias)
+    }
+  })
+})
+
+describe('no subBlock id carries two different defaults', () => {
+  it('every duplicated id agrees on its seeded value', () => {
+    const byId = new Map<string, Set<string>>()
+
+    for (const subBlock of CloudflareBlock.subBlocks) {
+      const value =
+        typeof subBlock.value === 'function'
+          ? (subBlock.value as (p: Record<string, never>) => unknown)({})
+          : (subBlock.defaultValue ?? null)
+
+      const seen = byId.get(subBlock.id) ?? new Set<string>()
+      seen.add(JSON.stringify(value ?? null))
+      byId.set(subBlock.id, seen)
+    }
+
+    const colliding = [...byId.entries()]
+      .filter(([, values]) => values.size > 1)
+      .map(([id, values]) => `${id}: ${[...values].join(' vs ')}`)
+
+    expect(colliding).toEqual([])
+  })
+})