Skip to content

Commit 8ae1f80

Browse files
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.
1 parent 71683d0 commit 8ae1f80

2 files changed

Lines changed: 96 additions & 64 deletions

File tree

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,40 @@ describe('preValidateCredentialInputs (hosted-tool blocks)', () => {
561561
expect(result.errors[0]).toMatchObject({ blockId: 'custom-1', field: 'serviceKey' })
562562
})
563563

564+
it('uses same-batch state for nested children (provider set earlier, apiKey set later)', async () => {
565+
const operations = [
566+
{
567+
operation_type: 'add' as const,
568+
block_id: 'loop-1',
569+
params: {
570+
type: 'loop',
571+
inputs: {},
572+
nestedNodes: {
573+
'video-child': { type: 'video_generator_v3', inputs: { provider: 'falai' } },
574+
},
575+
},
576+
},
577+
{
578+
operation_type: 'edit' as const,
579+
block_id: 'loop-1',
580+
params: {
581+
nestedNodes: {
582+
'video-child': { type: 'video_generator_v3', inputs: { apiKey: 'test-key' } },
583+
},
584+
},
585+
},
586+
]
587+
588+
const result = await preValidateCredentialInputs(operations, ctx)
589+
590+
const nested = result.filteredOperations[1]?.params?.nestedNodes as
591+
| Record<string, { inputs?: Record<string, unknown> }>
592+
| undefined
593+
expect(nested?.['video-child']?.inputs?.apiKey).toBeUndefined()
594+
expect(result.errors).toHaveLength(1)
595+
expect(result.errors[0]).toMatchObject({ blockId: 'video-child', field: 'apiKey' })
596+
})
597+
564598
it('uses same-batch state: a type-less apiKey edit after an earlier op makes the block hosted', async () => {
565599
// op1 switches provider to falai (hosted); op2 (type-less) sets apiKey. op2 must see op1's
566600
// provider, not the stale snapshot (runway), and strip the key.

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

Lines changed: 62 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,80 +1399,78 @@ export async function preValidateCredentialInputs(
13991399
const batchBlockType = new Map<string, string | undefined>()
14001400
const batchBlockValues = new Map<string, Record<string, unknown>>()
14011401

1402-
operations.forEach((op, opIndex) => {
1403-
const inputs = asRecord(op.params?.inputs)
1404-
1405-
// Effective block type: this op's type, else the type left by an earlier batch op, else the
1402+
// Resolve a single block (top-level or nested) against batch + snapshot state and run the
1403+
// collectors. `stateKey` keys the batch/snapshot lookup (the block's own id, including nested
1404+
// children); `reportBlockId`/`nestedBlockId` are how the strip surfaces it to the agent. Both
1405+
// paths route through here so they can't drift (the source of the earlier nested-vs-main gap).
1406+
const collectForBlock = (
1407+
opIndex: number,
1408+
stateKey: string,
1409+
reportBlockId: string,
1410+
rawType: string | undefined,
1411+
inputs: Record<string, unknown> | undefined,
1412+
nestedBlockId?: string
1413+
) => {
1414+
// Effective type: this op's type, else the type left by an earlier batch op, else the
14061415
// snapshot. Edit ops omit `type`, so without this an apiKey-only edit would skip stripping.
1407-
const priorType = batchBlockType.has(op.block_id)
1408-
? batchBlockType.get(op.block_id)
1409-
: (snapshotBlock(op.block_id)?.type as string | undefined)
1410-
const opBlockType = (op.params?.type as string | undefined) ?? priorType
1411-
batchBlockType.set(op.block_id, opBlockType)
1412-
1413-
// Process main block inputs
1414-
if (inputs && opBlockType) {
1415-
const blockConfig = getBlock(opBlockType)
1416-
if (blockConfig) {
1417-
collectCredentialInputs(blockConfig, inputs, opIndex, op.block_id, opBlockType)
1418-
1419-
// Both hosted collectors no-op off hosted Sim, so only reconstruct the effective inputs
1420-
// (prior batch/snapshot values overlaid with this op's delta) when it can matter.
1421-
if (isHosted) {
1422-
const priorValues =
1423-
batchBlockValues.get(op.block_id) ??
1424-
buildSubBlockValues(
1425-
(snapshotBlock(op.block_id)?.subBlocks as Record<string, { value?: unknown }>) ?? {}
1426-
)
1427-
const toolParams = { ...priorValues, ...inputs }
1428-
batchBlockValues.set(op.block_id, toolParams)
1429-
const modelValue = toolParams.model as string | undefined
1430-
collectHostedApiKeyInput(inputs, modelValue, opIndex, op.block_id, opBlockType)
1431-
collectHostedToolApiKeyInput(
1432-
blockConfig,
1433-
inputs,
1434-
toolParams,
1435-
opIndex,
1436-
op.block_id,
1437-
opBlockType
1438-
)
1439-
}
1440-
}
1416+
const priorType = batchBlockType.has(stateKey)
1417+
? batchBlockType.get(stateKey)
1418+
: (snapshotBlock(stateKey)?.type as string | undefined)
1419+
const blockType = rawType ?? priorType
1420+
batchBlockType.set(stateKey, blockType)
1421+
1422+
if (!inputs || !blockType) return
1423+
const blockConfig = getBlock(blockType)
1424+
if (!blockConfig) return
1425+
1426+
collectCredentialInputs(blockConfig, inputs, opIndex, reportBlockId, blockType, nestedBlockId)
1427+
1428+
// Both hosted collectors no-op off hosted Sim, so only reconstruct the effective inputs
1429+
// (prior batch/snapshot values overlaid with this op's delta) when it can matter.
1430+
if (isHosted) {
1431+
const priorValues =
1432+
batchBlockValues.get(stateKey) ??
1433+
buildSubBlockValues(
1434+
(snapshotBlock(stateKey)?.subBlocks as Record<string, { value?: unknown }>) ?? {}
1435+
)
1436+
const toolParams = { ...priorValues, ...inputs }
1437+
batchBlockValues.set(stateKey, toolParams)
1438+
const modelValue = toolParams.model as string | undefined
1439+
collectHostedApiKeyInput(inputs, modelValue, opIndex, reportBlockId, blockType, nestedBlockId)
1440+
collectHostedToolApiKeyInput(
1441+
blockConfig,
1442+
inputs,
1443+
toolParams,
1444+
opIndex,
1445+
reportBlockId,
1446+
blockType,
1447+
nestedBlockId
1448+
)
14411449
}
1450+
}
1451+
1452+
operations.forEach((op, opIndex) => {
1453+
collectForBlock(
1454+
opIndex,
1455+
op.block_id,
1456+
op.block_id,
1457+
op.params?.type as string | undefined,
1458+
asRecord(op.params?.inputs)
1459+
)
14421460

1443-
// Process nested nodes (blocks inside loop/parallel containers)
1461+
// Nested nodes (blocks inside loop/parallel containers) — keyed by their own child id so they
1462+
// get the same batch/snapshot resolution as top-level blocks.
14441463
const nestedNodes = op.params?.nestedNodes as
14451464
| Record<string, Record<string, unknown>>
14461465
| undefined
14471466
if (nestedNodes) {
14481467
Object.entries(nestedNodes).forEach(([childId, childBlock]) => {
1449-
const childType = childBlock.type as string | undefined
1450-
const childInputs = childBlock.inputs as Record<string, unknown> | undefined
1451-
if (!childType || !childInputs) return
1452-
1453-
const childBlockConfig = getBlock(childType)
1454-
if (!childBlockConfig) return
1455-
1456-
// Collect credentials from nested block
1457-
collectCredentialInputs(
1458-
childBlockConfig,
1459-
childInputs,
1460-
opIndex,
1461-
op.block_id,
1462-
childType,
1463-
childId
1464-
)
1465-
1466-
// Check for apiKey inputs on hosted models in nested block
1467-
const modelValue = childInputs.model as string | undefined
1468-
collectHostedApiKeyInput(childInputs, modelValue, opIndex, op.block_id, childType, childId)
1469-
collectHostedToolApiKeyInput(
1470-
childBlockConfig,
1471-
childInputs,
1472-
childInputs,
1468+
collectForBlock(
14731469
opIndex,
1470+
childId,
14741471
op.block_id,
1475-
childType,
1472+
childBlock.type as string | undefined,
1473+
asRecord(childBlock.inputs),
14761474
childId
14771475
)
14781476
})

0 commit comments

Comments
 (0)