fix(copilot): strip hosted apiKey on type-less edit ops + guard hosting.enabled (#5220)

* fix(copilot): strip hosted apiKey on type-less edit ops + guard hosting.enabled

The hosted-apiKey strip in preValidateCredentialInputs was gated on op.params.type, but edit ops omit type (they carry only changed inputs). An apiKey-only edit on a hosted-tool block therefore skipped both the model and tool strip paths, so a copilot-authored key persisted and disabled hosted-key injection.

Resolve the block type from workflowState when the op omits it, so type-less edits run through the strip. Also guard tool.hosting.enabled() in try/catch like the tool selector. Add a regression test mirroring the observed failure (edit op with only apiKey, type+provider resolved from workflow state).

* improvement(copilot): consolidate hosted-apiKey workflowState reads, gate off-hosted

Quality cleanup (no behavior change): the main loop read workflowState.blocks[id] three times (block type, model fallback, toolParams merge). Collapse to one existingBlock lookup + one buildSubBlockValues; derive modelValue from the merged toolParams (drops the model-fallback ladder); gate the reconstruction on isHosted so buildSubBlockValues + spreads no longer run off-hosted or for blocks the collectors skip. Add an asRecord helper to cut repeated casts.

* fix(copilot): resolve hosted-key strip against same-batch state; strip on enabled throw

Addresses review on #5220:
- Batch-aware block state: a later type-less edit now sees type/provider changed by an earlier op in the same edit_workflow request (was reading the stale initial snapshot), so a key can't survive on a block an earlier op just made hosted.
- hosting.enabled() throwing now fails toward treating the key as managed (strip) instead of preserving it, since the hosting state is unknown on throw.

* fix(copilot): unify hosted-key strip resolution across top-level and nested blocks

Greptile flagged that the batch/snapshot state reconstruction was applied only to top-level blocks; nested loop/parallel children still used raw childInputs, so a same-batch provider/model change on a nested child followed by a type-less apiKey edit could leave the key. Route both paths through one collectForBlock helper keyed by the block's own id (incl. nested children), so they share batch accumulation + snapshot enrichment and can't drift. Test added for the nested same-batch case.

* fix(copilot): recurse nestedNodes so grandchild hosted keys are stripped

The collection loop and the strip/credential-removal loops only handled the first nestedNodes level, but the apply path processes nestedNodes recursively (loop/parallel children can themselves contain nestedNodes). A hosted key on a grandchild (e.g. loop-in-loop) survived. Collection now walks the nestedNodes tree recursively; the strip/removal loops locate a descendant's inputs at any depth via findNestedInputs. Test added for a two-level-deep grandchild.

* fix(copilot): decide hosted-key strip against final batch state (two-pass)

Forward-only accumulation missed reverse order (apiKey set in an early op, block made hosted by a later op) and let an earlier bogus/empty type poison snapshot fallback. Replace it with a two-pass approach: pass 1 folds every op (and nested descendant) into each block's FINAL effective type+values for the batch; pass 2 strips the managed fields each op sets, judged against that final state. Order-independent, any nesting depth, and empty/invalid types no longer block fallback (validType guard). Tests added for reverse order and bogus-type cases.

* fix(copilot): tool selector throw falls back to access tools (fail toward strip)

Symmetric with the enabled-throw fix: when tools.config.tool throws on partial params, scan all access tools instead of returning, so a hosted key can't slip through. Test added.

* fix(copilot): only fold registry-known types into final batch state

Pass 1 recorded any non-empty type into finalType, but apply skips type changes to unknown types (keeps the existing block). An unknown type on an earlier op could poison a later type-less apiKey edit. Only advance finalType to a getBlock-resolvable type so the fallback matches what apply persists. Test covers empty and unknown types.

Author: TheodoreSpeaks

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts

@@ -114,6 +114,31 @@ const imageBlockConfig = {
   tools: { access: ['image_generate'], config: { tool: () => 'image_generate' } },
 }
 
+// Tool whose hosting.enabled predicate throws — used to assert fail-toward-strip behavior.
+const throwGateBlockConfig = {
+  type: 'throw_gate_block',
+  name: 'Throw Gate Block',
+  outputs: {},
+  subBlocks: [{ id: 'provider', type: 'dropdown' }],
+  tools: { access: ['throw_gate_tool'], config: { tool: () => 'throw_gate_tool' } },
+}
+
+// Block whose tool selector throws — should fall back to scanning access tools (video_falai).
+const throwSelectorBlockConfig = {
+  type: 'throw_selector_block',
+  name: 'Throw Selector Block',
+  outputs: {},
+  subBlocks: [{ id: 'provider', type: 'dropdown' }],
+  tools: {
+    access: ['video_falai'],
+    config: {
+      tool: () => {
+        throw new Error('selector boom')
+      },
+    },
+  },
+}
+
 // Tool registry stand-in for the hosted-tool tests.
 const toolsByIdMock: Record<string, unknown> = {
   video_falai: { id: 'video_falai', hosting: { apiKeyParam: 'apiKey' } },
@@ -126,6 +151,15 @@ const toolsByIdMock: Record<string, unknown> = {
       enabled: (p: Record<string, unknown>) => p.provider === 'falai',
     },
   },
+  throw_gate_tool: {
+    id: 'throw_gate_tool',
+    hosting: {
+      apiKeyParam: 'apiKey',
+      enabled: () => {
+        throw new Error('boom')
+      },
+    },
+  },
 }
 
 vi.mock('@/blocks/registry', () => ({
@@ -150,7 +184,11 @@ vi.mock('@/blocks/registry', () => ({
                       ? customKeyBlockConfig
                       : type === 'image_generator_v2'
                         ? imageBlockConfig
-                        : undefined,
+                        : type === 'throw_gate_block'
+                          ? throwGateBlockConfig
+                          : type === 'throw_selector_block'
+                            ? throwSelectorBlockConfig
+                            : undefined,
 }))
 
 vi.mock('@/blocks/utils', () => ({
@@ -469,6 +507,31 @@ describe('preValidateCredentialInputs (hosted-tool blocks)', () => {
     expect(result.errors).toHaveLength(1)
   })
 
+  it('strips apiKey on a type-less edit op, resolving block type + provider from workflow state', async () => {
+    // Mirrors the real failure: agent edits only { apiKey } with no `type` restated.
+    const operations = [
+      {
+        operation_type: 'edit' as const,
+        block_id: 'video-1',
+        params: { inputs: { apiKey: 'test-api-key-12345' } },
+      },
+    ]
+    const workflowState = {
+      blocks: {
+        'video-1': {
+          type: 'video_generator_v3',
+          subBlocks: { provider: { value: 'falai' } },
+        },
+      },
+    }
+
+    const result = await preValidateCredentialInputs(operations, ctx, workflowState)
+
+    expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+    expect(result.errors[0]).toMatchObject({ blockId: 'video-1', field: 'apiKey' })
+  })
+
   it('strips apiKey on a hosted-tool block nested inside a loop', async () => {
     const operations = [
       {
@@ -516,6 +579,198 @@ describe('preValidateCredentialInputs (hosted-tool blocks)', () => {
     expect(result.errors[0]).toMatchObject({ blockId: 'custom-1', field: 'serviceKey' })
   })
 
+  it('strips apiKey on a grandchild block nested two levels deep (loop in loop)', async () => {
+    const operations = [
+      {
+        operation_type: 'add' as const,
+        block_id: 'outer-loop',
+        params: {
+          type: 'loop',
+          inputs: {},
+          nestedNodes: {
+            'inner-loop': {
+              type: 'loop',
+              inputs: {},
+              nestedNodes: {
+                'video-child': {
+                  type: 'video_generator_v3',
+                  inputs: { provider: 'falai', apiKey: '{{FAL_API_KEY}}' },
+                },
+              },
+            },
+          },
+        },
+      },
+    ]
+
+    const result = await preValidateCredentialInputs(operations, ctx)
+
+    const innerInputs = (
+      (result.filteredOperations[0]?.params?.nestedNodes as Record<string, any>)?.['inner-loop']
+        ?.nestedNodes as Record<string, { inputs?: Record<string, unknown> }>
+    )?.['video-child']?.inputs
+    expect(innerInputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+    expect(result.errors[0]).toMatchObject({ blockId: 'video-child', field: 'apiKey' })
+  })
+
+  it('uses same-batch state for nested children (provider set earlier, apiKey set later)', async () => {
+    const operations = [
+      {
+        operation_type: 'add' as const,
+        block_id: 'loop-1',
+        params: {
+          type: 'loop',
+          inputs: {},
+          nestedNodes: {
+            'video-child': { type: 'video_generator_v3', inputs: { provider: 'falai' } },
+          },
+        },
+      },
+      {
+        operation_type: 'edit' as const,
+        block_id: 'loop-1',
+        params: {
+          nestedNodes: {
+            'video-child': { type: 'video_generator_v3', inputs: { apiKey: 'test-key' } },
+          },
+        },
+      },
+    ]
+
+    const result = await preValidateCredentialInputs(operations, ctx)
+
+    const nested = result.filteredOperations[1]?.params?.nestedNodes as
+      | Record<string, { inputs?: Record<string, unknown> }>
+      | undefined
+    expect(nested?.['video-child']?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+    expect(result.errors[0]).toMatchObject({ blockId: 'video-child', field: 'apiKey' })
+  })
+
+  it('strips a key set before a later op makes the block hosted (reverse batch order)', async () => {
+    // op1 sets apiKey while the block is still non-hosted (runway); op2 later flips it to falai.
+    // Deciding against final state must still strip op1's key.
+    const operations = [
+      {
+        operation_type: 'edit' as const,
+        block_id: 'video-1',
+        params: { inputs: { apiKey: '{{FAL_API_KEY}}' } },
+      },
+      {
+        operation_type: 'edit' as const,
+        block_id: 'video-1',
+        params: { inputs: { provider: 'falai' } },
+      },
+    ]
+    const workflowState = {
+      blocks: {
+        'video-1': { type: 'video_generator_v3', subBlocks: { provider: { value: 'runway' } } },
+      },
+    }
+
+    const result = await preValidateCredentialInputs(operations, ctx, workflowState)
+
+    expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+    expect(result.errors[0]).toMatchObject({ blockId: 'video-1', field: 'apiKey' })
+  })
+
+  it.each([{ type: '' }, { type: 'totally_unknown_type' }])(
+    'does not let an invalid type (%o) on an earlier op block stripping on a later edit',
+    async ({ type }) => {
+      const operations = [
+        {
+          operation_type: 'edit' as const,
+          block_id: 'video-1',
+          params: { type, inputs: { prompt: 'x' } },
+        },
+        {
+          operation_type: 'edit' as const,
+          block_id: 'video-1',
+          params: { inputs: { apiKey: '{{FAL_API_KEY}}' } },
+        },
+      ]
+      const workflowState = {
+        blocks: {
+          'video-1': { type: 'video_generator_v3', subBlocks: { provider: { value: 'falai' } } },
+        },
+      }
+
+      const result = await preValidateCredentialInputs(operations, ctx, workflowState)
+
+      expect(result.filteredOperations[1]?.params?.inputs?.apiKey).toBeUndefined()
+      expect(result.errors).toHaveLength(1)
+    }
+  )
+
+  it('uses same-batch state: a type-less apiKey edit after an earlier op makes the block hosted', async () => {
+    // op1 switches provider to falai (hosted); op2 (type-less) sets apiKey. op2 must see op1's
+    // provider, not the stale snapshot (runway), and strip the key.
+    const operations = [
+      {
+        operation_type: 'edit' as const,
+        block_id: 'video-1',
+        params: { inputs: { provider: 'falai' } },
+      },
+      {
+        operation_type: 'edit' as const,
+        block_id: 'video-1',
+        params: { inputs: { apiKey: 'test-api-key-12345' } },
+      },
+    ]
+    const workflowState = {
+      blocks: {
+        'video-1': {
+          type: 'video_generator_v3',
+          subBlocks: { provider: { value: 'runway' } },
+        },
+      },
+    }
+
+    const result = await preValidateCredentialInputs(operations, ctx, workflowState)
+
+    expect(result.filteredOperations[1]?.params?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+    expect(result.errors[0]).toMatchObject({ blockId: 'video-1', field: 'apiKey' })
+  })
+
+  it('strips apiKey when the tool selector throws (falls back to access tools)', async () => {
+    const operations = [
+      {
+        operation_type: 'add' as const,
+        block_id: 'sel-1',
+        params: {
+          type: 'throw_selector_block',
+          inputs: { provider: 'falai', apiKey: 'user-key' },
+        },
+      },
+    ]
+
+    const result = await preValidateCredentialInputs(operations, ctx)
+
+    expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+  })
+
+  it('strips apiKey when a tool hosting enabled predicate throws (fail toward stripping)', async () => {
+    const operations = [
+      {
+        operation_type: 'add' as const,
+        block_id: 'gate-1',
+        params: {
+          type: 'throw_gate_block',
+          inputs: { provider: 'whatever', apiKey: 'user-key' },
+        },
+      },
+    ]
+
+    const result = await preValidateCredentialInputs(operations, ctx)
+
+    expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
+    expect(result.errors).toHaveLength(1)
+  })
+
   it('preserves apiKey on self-hosted deployments (isHosted false)', async () => {
     mockEnvFlags.isHosted = false
     const operations = [