Skip to content

Commit d3a5c5b

Browse files
author
Vikhyath Mondreti
committed
address comments
1 parent a2805bb commit d3a5c5b

7 files changed

Lines changed: 203 additions & 31 deletions

File tree

‎apps/sim/executor/execution/block-executor.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,55 @@ describe('BlockExecutor streaming pump', () => {
11351135
expect(registry.getActiveMatches()).toEqual([])
11361136
})
11371137

1138+
it('carries echoed raw-boundary secret provenance on terminal errors only', async () => {
1139+
const promptSecret = 'x'
1140+
const apiKey = 'provider-credential-secret'
1141+
const handler: BlockHandler = {
1142+
canHandle: () => true,
1143+
execute: async (blockContext) => {
1144+
const sourceRegistry = blockContext.resolvedSecretTraceRegistry
1145+
blockContext.errorResolvedSecretTraceRegistry = sourceRegistry?.forkForInputPaths([
1146+
['apiKey'],
1147+
])
1148+
blockContext.resolvedSecretTraceRegistry = sourceRegistry?.forkForInputPaths([])
1149+
throw new Error(`Provider rejected ${apiKey}`)
1150+
},
1151+
}
1152+
const { executor, block, state } = createExecutor(handler)
1153+
block.config.params = {
1154+
systemPrompt: '{{PROMPT_TOKEN}}',
1155+
apiKey: '{{API_KEY}}',
1156+
}
1157+
const ctx = createContext(state)
1158+
const registry = new ResolvedSecretTraceRegistry([
1159+
{
1160+
name: 'PROMPT_TOKEN',
1161+
plaintext: promptSecret,
1162+
encryptedValue: 'encrypted-prompt-token',
1163+
},
1164+
{ name: 'API_KEY', plaintext: apiKey, encryptedValue: 'encrypted-api-key' },
1165+
])
1166+
ctx.environmentVariables = { PROMPT_TOKEN: promptSecret, API_KEY: apiKey }
1167+
ctx.resolvedSecretTraceRegistry = registry
1168+
1169+
await expect(executor.execute(ctx, createNode(block), block)).rejects.toThrow(
1170+
`Agent: Provider rejected ${apiKey}`
1171+
)
1172+
1173+
expect(ctx.blockLogs[0]).toMatchObject({
1174+
input: { systemPrompt: '{{PROMPT_TOKEN}}', apiKey: '[REDACTED]' },
1175+
output: { error: `Provider rejected ${apiKey}` },
1176+
})
1177+
const expectedProvenance = {
1178+
version: 1,
1179+
complete: true,
1180+
entries: [{ name: 'API_KEY', encryptedValue: 'encrypted-api-key' }],
1181+
}
1182+
expect(state.getBlockState(block.id)?.resolvedSecretTraceProvenance).toEqual(expectedProvenance)
1183+
expect(ctx.blockLogs[0]?.displayResolvedSecretTraceProvenance).toEqual(expectedProvenance)
1184+
expect(registry.getActiveMatches()).toEqual([])
1185+
})
1186+
11381187
it('suppresses an incomplete display input without failing block execution', async () => {
11391188
const handler: BlockHandler = {
11401189
canHandle: () => true,

‎apps/sim/executor/execution/block-executor.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,16 @@ export class BlockExecutor {
250250
normalizeStringArray(blockCtx.selectedOutputs)
251251
)
252252
} catch (streamError) {
253-
blockCtx.resolvedSecretTraceRegistry =
254-
blockCtx.resolvedSecretTraceRegistry?.forkForPropagatedEntries()
253+
const resultRegistry = blockCtx.resolvedSecretTraceRegistry
254+
const diagnosticRegistry = streamingExec.diagnosticResolvedSecretTraceRegistry
255+
const errorRegistry = diagnosticRegistry
256+
? diagnosticRegistry.forkForToolCall()
257+
: resultRegistry?.forkForToolCall()
258+
if (errorRegistry && resultRegistry && resultRegistry !== diagnosticRegistry) {
259+
errorRegistry.mergeToolCallRegistry(resultRegistry)
260+
}
261+
blockCtx.errorResolvedSecretTraceRegistry = errorRegistry
262+
blockCtx.resolvedSecretTraceRegistry = resultRegistry?.forkForPropagatedEntries()
255263
// Timeout / drain failures may still have projected answer text — keep it
256264
// for the failed block output so logs match what the client already saw.
257265
streamingPartialOutput = streamingExec.execution?.output
@@ -575,8 +583,8 @@ export class BlockExecutor {
575583
}
576584
}
577585

578-
const errorOutputProvenance =
579-
ctx.resolvedSecretTraceRegistry?.exportCommittedProvenanceForValue(errorOutput)
586+
const errorRegistry = ctx.errorResolvedSecretTraceRegistry ?? ctx.resolvedSecretTraceRegistry
587+
const errorOutputProvenance = errorRegistry?.exportCommittedProvenanceForValue(errorOutput)
580588
this.setNodeOutput(node, errorOutput, duration, errorOutputProvenance)
581589

582590
if (blockLog) {
@@ -592,8 +600,11 @@ export class BlockExecutor {
592600
}
593601
}
594602

595-
const diagnosticRegistry = inputDisplayRegistry?.forkForToolCall()
603+
const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry
604+
? ctx.errorResolvedSecretTraceRegistry
605+
: inputDisplayRegistry?.forkForToolCall()
596606
if (
607+
!ctx.errorResolvedSecretTraceRegistry &&
597608
diagnosticRegistry &&
598609
ctx.resolvedSecretTraceRegistry &&
599610
ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry
@@ -620,7 +631,7 @@ export class BlockExecutor {
620631
: undefined
621632
const displayOutput = filterOutputForLog(block.metadata?.id || '', errorOutput, { block })
622633
const displayInput = this.projectInputsForDisplay(input, block, inputDisplayRegistry)
623-
const displayProvenance = ctx.resolvedSecretTraceRegistry?.exportCommittedProvenanceForValue({
634+
const displayProvenance = errorRegistry?.exportCommittedProvenanceForValue({
624635
input: displayInput,
625636
output: displayOutput,
626637
})

‎apps/sim/executor/handlers/agent/agent-handler.test.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,30 +1191,38 @@ describe('AgentBlockHandler', () => {
11911191
).toEqual({ version: 1, complete: true, entries: [] })
11921192
})
11931193

1194-
it('keeps input provenance active for provider error diagnostics', async () => {
1194+
it('keeps only raw provider inputs active for provider error diagnostics', async () => {
11951195
const plaintext = 'provider-credential-secret'
11961196
const registry = new ResolvedSecretTraceRegistry([
1197-
{ name: 'TOKEN', plaintext, encryptedValue: 'encrypted-token' },
1197+
{ name: 'API_KEY', plaintext, encryptedValue: 'encrypted-api-key' },
1198+
{ name: 'PROMPT_TOKEN', plaintext: 'x', encryptedValue: 'encrypted-prompt-token' },
11981199
])
1199-
registry.recordResolvedAtInputPath('TOKEN', plaintext, ['systemPrompt'])
1200-
registry.recordResolvedInputProjection(['systemPrompt'], `Use ${plaintext}`, 'Use {{TOKEN}}')
1200+
registry.recordResolvedAtInputPath('API_KEY', plaintext, ['apiKey'])
1201+
registry.recordResolvedInputProjection(['apiKey'], plaintext, '{{API_KEY}}')
1202+
registry.recordResolvedAtInputPath('PROMPT_TOKEN', 'x', ['systemPrompt'])
1203+
registry.recordResolvedInputProjection(['systemPrompt'], 'Use x', 'Use {{PROMPT_TOKEN}}')
12011204
mockContext.resolvedSecretTraceRegistry = registry
12021205
mockExecuteProviderRequest.mockRejectedValueOnce(new Error(`Provider rejected ${plaintext}`))
12031206
const inputs = {
12041207
model: 'gpt-4o',
1205-
systemPrompt: `Use ${plaintext}`,
1208+
systemPrompt: 'Use x',
12061209
userPrompt: 'Continue',
1210+
apiKey: plaintext,
12071211
}
12081212

12091213
await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
12101214
`Provider rejected ${plaintext}`
12111215
)
12121216

1213-
expect(inputs.systemPrompt).toBe(`Use ${plaintext}`)
1217+
expect(inputs).toMatchObject({ systemPrompt: 'Use x', apiKey: plaintext })
12141218
expect(mockContext.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([])
1219+
expect(mockContext.errorResolvedSecretTraceRegistry?.getActiveMatches()).toEqual([
1220+
{ plaintext, replacement: '{{API_KEY}}' },
1221+
])
12151222
const logged = JSON.stringify(mockAgentLogger.error.mock.calls)
12161223
expect(logged).not.toContain(plaintext)
1217-
expect(logged).toContain('Provider rejected {{TOKEN}}')
1224+
expect(logged).toContain('Provider rejected {{API_KEY}}')
1225+
expect(logged).not.toContain('PROMPT_TOKEN')
12181226
})
12191227

12201228
it('projects exact message call arguments without mutating protocol structure or raw input', async () => {

‎apps/sim/executor/handlers/agent/agent-handler.ts‎

Lines changed: 66 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,26 @@ import { getToolAsync } from '@/tools/utils.server'
9494

9595
const logger = createLogger('AgentBlockHandler')
9696
const MODEL_SAFE_RESPONSE_FORMAT_NAME = 'response_schema'
97+
const AGENT_RAW_PROVIDER_ERROR_INPUT_PATHS: readonly ResolvedSecretInputPath[] = [
98+
['model'],
99+
['temperature'],
100+
['maxTokens'],
101+
['apiKey'],
102+
['azureEndpoint'],
103+
['azureApiVersion'],
104+
['vertexProject'],
105+
['vertexLocation'],
106+
['vertexCredential'],
107+
['bedrockAccessKeyId'],
108+
['bedrockSecretKey'],
109+
['bedrockRegion'],
110+
['reasoningEffort'],
111+
['verbosity'],
112+
['thinkingLevel'],
113+
['promptCaching'],
114+
['previousInteractionId'],
115+
]
116+
const AGENT_MEMORY_ERROR_INPUT_PATHS: readonly ResolvedSecretInputPath[] = [['conversationId']]
97117

98118
interface IndexedToolInput {
99119
tool: ToolInput
@@ -193,6 +213,10 @@ export class AgentBlockHandler implements BlockHandler {
193213
block: SerializedBlock,
194214
inputs: AgentInputs
195215
): Promise<BlockOutput | StreamingExecution> {
216+
const providerErrorRegistry = ctx.resolvedSecretTraceRegistry?.forkForInputPaths(
217+
AGENT_RAW_PROVIDER_ERROR_INPUT_PATHS
218+
)
219+
ctx.errorResolvedSecretTraceRegistry = providerErrorRegistry
196220
const toolIndexByRef = new Map<ToolInput, number>(
197221
(inputs.tools || []).map((tool, index) => [tool, index] as const)
198222
)
@@ -371,7 +395,8 @@ export class AgentBlockHandler implements BlockHandler {
371395
providerRequest,
372396
block,
373397
responseFormat,
374-
resultRegistry
398+
resultRegistry,
399+
providerErrorRegistry
375400
)
376401
if (resultRegistry) ctx.resolvedSecretTraceRegistry = resultRegistry
377402

@@ -385,24 +410,30 @@ export class AgentBlockHandler implements BlockHandler {
385410

386411
if (this.isStreamingExecution(result)) {
387412
const streamingResult = result as StreamingExecution
388-
streamingResult.diagnosticResolvedSecretTraceRegistry = settledInputRegistry
413+
streamingResult.diagnosticResolvedSecretTraceRegistry = providerErrorRegistry
414+
const memoryErrorRegistry = settledInputRegistry?.forkForInputPaths(
415+
AGENT_MEMORY_ERROR_INPUT_PATHS
416+
)
389417
if (filteredInputs.memoryType && filteredInputs.memoryType !== 'none') {
390418
return this.wrapStreamForMemoryPersistence(
391419
ctx,
392420
filteredInputs,
393421
streamingResult,
394-
settledInputRegistry
422+
memoryErrorRegistry
395423
)
396424
}
397425
return streamingResult
398426
}
399427

400428
if (filteredInputs.memoryType && filteredInputs.memoryType !== 'none') {
429+
const memoryErrorRegistry = settledInputRegistry?.forkForInputPaths(
430+
AGENT_MEMORY_ERROR_INPUT_PATHS
431+
)
401432
await this.persistResponseToMemory(
402433
ctx,
403434
filteredInputs,
404435
result as BlockOutput,
405-
settledInputRegistry
436+
memoryErrorRegistry
406437
)
407438
}
408439

@@ -2297,7 +2328,8 @@ export class AgentBlockHandler implements BlockHandler {
22972328
providerRequest: any,
22982329
block: SerializedBlock,
22992330
responseFormat: any,
2300-
modelRuntimeRegistry: ResolvedSecretTraceRegistry | undefined
2331+
modelRuntimeRegistry: ResolvedSecretTraceRegistry | undefined,
2332+
providerErrorRegistry: ResolvedSecretTraceRegistry | undefined
23012333
): Promise<BlockOutput | StreamingExecution> {
23022334
const providerId = providerRequest.provider
23032335
const model = providerRequest.model
@@ -2369,14 +2401,13 @@ export class AgentBlockHandler implements BlockHandler {
23692401

23702402
return this.processProviderResponse(response, block, responseFormat, ctx)
23712403
} catch (error) {
2372-
const sourceRegistry = ctx.resolvedSecretTraceRegistry
2373-
if (sourceRegistry && modelRuntimeRegistry && sourceRegistry !== modelRuntimeRegistry) {
2374-
const diagnosticRegistry = sourceRegistry.forkForToolCall()
2375-
diagnosticRegistry.mergeToolCallRegistry(modelRuntimeRegistry)
2376-
ctx.resolvedSecretTraceRegistry = diagnosticRegistry
2377-
}
2404+
const errorRegistry = this.createErrorRegistry(providerErrorRegistry, modelRuntimeRegistry)
2405+
ctx.errorResolvedSecretTraceRegistry = errorRegistry
2406+
const diagnosticCtx = errorRegistry
2407+
? { ...ctx, resolvedSecretTraceRegistry: errorRegistry }
2408+
: ctx
23782409
try {
2379-
this.handleExecutionError(error, providerStartTime, providerId, model, ctx, block)
2410+
this.handleExecutionError(error, providerStartTime, providerId, model, diagnosticCtx, block)
23802411
} finally {
23812412
if (modelRuntimeRegistry) {
23822413
ctx.resolvedSecretTraceRegistry = modelRuntimeRegistry.forkForPropagatedEntries()
@@ -2386,6 +2417,17 @@ export class AgentBlockHandler implements BlockHandler {
23862417
}
23872418
}
23882419

2420+
private createErrorRegistry(
2421+
inputRegistry: ResolvedSecretTraceRegistry | undefined,
2422+
resultRegistry: ResolvedSecretTraceRegistry | undefined
2423+
): ResolvedSecretTraceRegistry | undefined {
2424+
const errorRegistry = inputRegistry?.forkForToolCall() ?? resultRegistry?.forkForToolCall()
2425+
if (errorRegistry && resultRegistry && resultRegistry !== inputRegistry) {
2426+
errorRegistry.mergeToolCallRegistry(resultRegistry)
2427+
}
2428+
return errorRegistry
2429+
}
2430+
23892431
private handleExecutionError(
23902432
error: any,
23912433
startTime: number,
@@ -2452,8 +2494,12 @@ export class AgentBlockHandler implements BlockHandler {
24522494
try {
24532495
await memoryService.appendToMemory(ctx, inputs, { role: 'assistant', content })
24542496
} catch (error) {
2455-
const diagnosticCtx = diagnosticRegistry
2456-
? { ...ctx, resolvedSecretTraceRegistry: diagnosticRegistry }
2497+
const memoryErrorRegistry = this.createErrorRegistry(
2498+
diagnosticRegistry,
2499+
ctx.resolvedSecretTraceRegistry
2500+
)
2501+
const diagnosticCtx = memoryErrorRegistry
2502+
? { ...ctx, resolvedSecretTraceRegistry: memoryErrorRegistry }
24572503
: ctx
24582504
logger.error(
24592505
'Failed to persist streaming response',
@@ -2485,8 +2531,12 @@ export class AgentBlockHandler implements BlockHandler {
24852531
workflowId: ctx.workflowId,
24862532
})
24872533
} catch (error) {
2488-
const diagnosticCtx = diagnosticRegistry
2489-
? { ...ctx, resolvedSecretTraceRegistry: diagnosticRegistry }
2534+
const memoryErrorRegistry = this.createErrorRegistry(
2535+
diagnosticRegistry,
2536+
ctx.resolvedSecretTraceRegistry
2537+
)
2538+
const diagnosticCtx = memoryErrorRegistry
2539+
? { ...ctx, resolvedSecretTraceRegistry: memoryErrorRegistry }
24902540
: ctx
24912541
logger.error(
24922542
'Failed to persist response to memory',

‎apps/sim/executor/handlers/mothership/mothership-handler.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,43 @@ describe('MothershipBlockHandler', () => {
489489
expect(registry.markIncomplete).not.toHaveBeenCalled()
490490
})
491491

492+
it('keeps declared JSON error provenance separate from normal result provenance', async () => {
493+
const registry = createTraceRegistryMock()
494+
context.resolvedSecretTraceRegistry = registry
495+
mockExtractAPIErrorMessage.mockResolvedValueOnce('secret-backed failure')
496+
fetchMock.mockResolvedValue(
497+
new Response(
498+
JSON.stringify({
499+
error: 'secret-backed failure',
500+
__resolvedSecretTraceProvenance: PRIVATE_PROVENANCE,
501+
}),
502+
{
503+
status: 502,
504+
headers: {
505+
'Content-Type': 'application/json',
506+
'x-sim-private-tool-metadata': PRIVATE_PROVENANCE_TYPE,
507+
},
508+
}
509+
)
510+
)
511+
512+
await expect(handler.execute(context, block, { prompt: 'Hello' })).rejects.toThrow(
513+
'Sim execution failed: secret-backed failure'
514+
)
515+
516+
expect(registry.importProvenanceForValue).toHaveBeenCalledWith(
517+
PRIVATE_PROVENANCE,
518+
expect.objectContaining({
519+
error: 'secret-backed failure',
520+
__resolvedSecretTraceProvenance: undefined,
521+
}),
522+
{ trusted: true }
523+
)
524+
expect(context.errorResolvedSecretTraceRegistry).toBeDefined()
525+
expect(context.errorResolvedSecretTraceRegistry).not.toBe(context.resolvedSecretTraceRegistry)
526+
expect(context.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([])
527+
})
528+
492529
it('imports provenance from a terminal NDJSON error without forcing structural fallback', async () => {
493530
const registry = createTraceRegistryMock()
494531
context.resolvedSecretTraceRegistry = registry
@@ -523,6 +560,8 @@ describe('MothershipBlockHandler', () => {
523560
{ trusted: true }
524561
)
525562
expect(registry.markIncomplete).not.toHaveBeenCalled()
563+
expect(context.errorResolvedSecretTraceRegistry).toBeDefined()
564+
expect(context.errorResolvedSecretTraceRegistry).not.toBe(context.resolvedSecretTraceRegistry)
526565
})
527566

528567
it('imports final provenance for selected-output streaming without adding it to output', async () => {
@@ -926,6 +965,7 @@ describe('MothershipBlockHandler', () => {
926965
).rejects.toThrow('Sim execution failed: Box')
927966

928967
expect(context.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([])
968+
expect(context.errorResolvedSecretTraceRegistry?.getActiveMatches()).toEqual([])
929969
})
930970

931971
it('forwards only enabled MCP tools and selected skills', async () => {

0 commit comments

Comments
 (0)