Skip to content

Commit c14bd4d

Browse files
committed
fix(agent): close the adversarial review's fallback findings
A routed sim-auto primary that fails now shows in the trace as the auto identity rather than the pool model, which is the name applyAutoModelLabel exists to hide. Per-row tuning resolves against the model the builder configured, the one the editor showed the fields for, so a row's value applies under sim-auto whatever pool model was routed. A fallback on another provider no longer receives the primary's Azure, Vertex, or Bedrock fields, which only that family reads.
1 parent 93abc4c commit c14bd4d

2 files changed

Lines changed: 77 additions & 13 deletions

File tree

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,32 @@ describe('AgentBlockHandler', () => {
757757
)
758758
})
759759

760+
it('leaves provider-family credentials off a fallback on another provider', async () => {
761+
mockExecuteProviderRequest
762+
.mockRejectedValueOnce(new Error('one'))
763+
.mockRejectedValueOnce(new Error('two'))
764+
.mockResolvedValueOnce(providerResponse('gpt-4o-mini'))
765+
766+
await handler.execute(mockContext, mockBlock, {
767+
...baseInputs,
768+
vertexCredential: 'vertex-secret',
769+
bedrockSecretKey: 'bedrock-secret',
770+
azureEndpoint: 'https://azure.example.com',
771+
fallbackModels: [{ model: 'claude-sonnet-5' }, { model: 'gpt-4o-mini' }],
772+
})
773+
774+
const [, crossProvider] = mockExecuteProviderRequest.mock.calls[1]
775+
const [, sameProvider] = mockExecuteProviderRequest.mock.calls[2]
776+
expect(crossProvider.vertexCredential).toBeUndefined()
777+
expect(crossProvider.bedrockSecretKey).toBeUndefined()
778+
expect(crossProvider.azureEndpoint).toBeUndefined()
779+
expect(JSON.stringify(crossProvider)).not.toMatch(
780+
/vertex-secret|bedrock-secret|azure\.example/
781+
)
782+
expect(sameProvider.bedrockSecretKey).toBe('bedrock-secret')
783+
expect(sameProvider.azureEndpoint).toBe('https://azure.example.com')
784+
})
785+
760786
it('ignores a row key the block did not store as a reference', async () => {
761787
mockExecuteProviderRequest
762788
.mockRejectedValueOnce(new Error('one'))
@@ -1112,17 +1138,22 @@ describe('AgentBlockHandler', () => {
11121138
it('keeps the fallback name when a routed sim-auto primary fails and a fallback answers', async () => {
11131139
mockExecuteProviderRequest
11141140
.mockRejectedValueOnce(new Error('pool model down'))
1115-
.mockResolvedValueOnce(providerResponse('gpt-4o-mini'))
1141+
.mockResolvedValueOnce(providerResponse('gpt-5.4-mini'))
1142+
const blockLog = openLog()
11161143

1117-
const result = (await handler.execute(mockContext, mockBlock, {
1144+
const result = (await handler.execute({ ...mockContext, blockLogs: [blockLog] }, mockBlock, {
11181145
model: SIM_AUTO_MODEL_ID,
11191146
systemPrompt: 'Be brief.',
11201147
userPrompt: 'Hello!',
1121-
fallbackModels: [{ model: 'gpt-4o-mini' }],
1148+
fallbackModels: [{ model: 'gpt-5.4-mini', reasoningEffort: 'low' }],
11221149
})) as { model: string }
11231150

1124-
expect(result.model).toBe('gpt-4o-mini')
1151+
expect(result.model).toBe('gpt-5.4-mini')
11251152
expect(mockExecuteProviderRequest).toHaveBeenCalledTimes(2)
1153+
/** The trace names the auto identity, never the pool model that was routed. */
1154+
expect(blockLog.modelFallbacks).toEqual([SIM_AUTO_MODEL_ID])
1155+
/** The row's tuning was set against the auto id in the editor, so it applies whatever was routed. */
1156+
expect(mockExecuteProviderRequest.mock.calls[1][1].reasoningEffort).toBe('low')
11261157
/** The auto identity preamble belongs to the pool model, not a named fallback. */
11271158
const systemText = (request: { messages?: Array<{ role: string; content: string }> }) =>
11281159
(request.messages ?? [])

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

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,14 +170,42 @@ function stripAutoPreamble(messages: Message[] | undefined): Message[] | undefin
170170
})
171171
}
172172

173+
/**
174+
* Block fields that only a provider family reads. A fallback on another provider
175+
* never needs them, so they are left off its request rather than handed to a
176+
* provider that has no use for a Bedrock secret or a Vertex credential.
177+
*/
178+
const PROVIDER_FAMILY_CREDENTIAL_FIELDS = [
179+
'azureEndpoint',
180+
'azureApiVersion',
181+
'vertexProject',
182+
'vertexLocation',
183+
'vertexCredential',
184+
'bedrockAccessKeyId',
185+
'bedrockSecretKey',
186+
'bedrockRegion',
187+
] as const satisfies ReadonlyArray<keyof AgentInputs>
188+
173189
/** One model in the order the block tries them; the primary carries the block's own key. */
174190
interface ModelCandidate extends FallbackModelCandidate {
175191
isPrimary: boolean
192+
/**
193+
* What the trace calls this model when it fails. A routed sim-auto primary
194+
* shows as the auto identity, since naming the pool model is the leak that
195+
* `applyAutoModelLabel` exists to close.
196+
*/
197+
traceName?: string
176198
}
177199

178200
interface ExecuteAcrossModelsConfig {
179201
candidates: ModelCandidate[]
180202
primaryModel: string
203+
/**
204+
* The model the builder configured, which is what the editor showed the
205+
* per-row tuning fields against. Under sim-auto that is the auto id, not the
206+
* pool model routed for this run, so a row's value applies whatever was routed.
207+
*/
208+
configuredModel: string
181209
primaryProviderId: string
182210
messages: Message[] | undefined
183211
/** Provider id to hydrated messages; seeded with the primary, filled per fallback provider. */
@@ -523,7 +551,12 @@ export class AgentBlockHandler implements BlockHandler {
523551
})
524552
}
525553
const candidates: ModelCandidate[] = [
526-
{ model, apiKey: modelInputs.apiKey, isPrimary: true },
554+
{
555+
model,
556+
apiKey: modelInputs.apiKey,
557+
isPrimary: true,
558+
...(autoRouting ? { traceName: SIM_AUTO_MODEL_ID } : {}),
559+
},
527560
...fallbackCandidates.map((candidate) => ({ ...candidate, isPrimary: false })),
528561
]
529562
const {
@@ -533,6 +566,7 @@ export class AgentBlockHandler implements BlockHandler {
533566
} = await this.executeAcrossModels(ctx, block, {
534567
candidates,
535568
primaryModel: model,
569+
configuredModel: autoRouting ? SIM_AUTO_MODEL_ID : model,
536570
primaryProviderId: providerId,
537571
messages: messagesWithInputFiles,
538572
hydratedByProvider,
@@ -2534,7 +2568,7 @@ export class AgentBlockHandler implements BlockHandler {
25342568
if (!candidate.isPrimary) {
25352569
const { adjustments, ...tuning } = resolveFallbackTuning(
25362570
candidate,
2537-
config.primaryModel,
2571+
config.configuredModel,
25382572
config.modelInputs
25392573
)
25402574
/**
@@ -2552,13 +2586,12 @@ export class AgentBlockHandler implements BlockHandler {
25522586
})
25532587
rowKey = undefined
25542588
}
2589+
const sameProvider = candidateProviderId === config.primaryProviderId
25552590
inputs = {
2556-
...config.modelInputs,
2557-
apiKey:
2558-
rowKey ??
2559-
(candidateProviderId === config.primaryProviderId
2560-
? config.modelInputs.apiKey
2561-
: undefined),
2591+
...(sameProvider
2592+
? config.modelInputs
2593+
: omit(config.modelInputs, [...PROVIDER_FAMILY_CREDENTIAL_FIELDS])),
2594+
apiKey: rowKey ?? (sameProvider ? config.modelInputs.apiKey : undefined),
25622595
previousInteractionId: undefined,
25632596
...(config.fallbackSystemPrompt !== undefined
25642597
? { systemPrompt: config.fallbackSystemPrompt }
@@ -2607,7 +2640,7 @@ export class AgentBlockHandler implements BlockHandler {
26072640
return { result, servedModel: candidate.model, resultRegistry }
26082641
} catch (error) {
26092642
lastError = error
2610-
failedModels.push(candidate.model)
2643+
failedModels.push(candidate.traceName ?? candidate.model)
26112644
if (!hasNext || ctx.abortSignal?.aborted || !isRetryableBlockError(error)) {
26122645
this.recordModelFallbacks(ctx, block, failedModels.slice(0, -1))
26132646
throw error

0 commit comments

Comments
 (0)