fix(agent): address the pre-landing review of retry-then-fallback

Review fixes: the executor judges the final try once per iteration; the
skip-warn is one helper; the fallback warn names the candidate position
rather than reusing `attempt`; the BlockLog.modelFallbacks doc matches the
final-try semantics; the viability check resolves a provider once through
the new providerRequiresFamilyCredentials; editor handlers read rows via a
ref so a keystroke in one row no longer re-renders every row, and tuning
options keep their identity across renders.

Two consistency fixes from the red-team pass: a sim-auto fallback takes
the projected system prompt rather than the raw input, and a row key is
honoured at runtime only when the block stored it as a whole {{NAME}}
reference, the one form the editor, the validator, and an export agree on.

Tests now cover the composed executor-plus-handler sequence (three tries on
the selected model, then each fallback once), the node-taking handler
signature, a fallback whose provider cannot take the attachments, the
per-provider hydration cache, a stop during a skipped candidate, the
un-primed stream when no candidate follows, a non-retryable failure on a
non-final try, a numeric tuning value, and an unresolved temperature.

Author: mzxchandra

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/model-fallback-list/model-fallback-list.tsx

@@ -1,6 +1,6 @@
 'use client'
 
-import { memo, useCallback, useMemo } from 'react'
+import { memo, useCallback, useEffect, useMemo, useRef } from 'react'
 import { Button, Combobox, type ComboboxOption, Label, Tooltip } from '@sim/emcn'
 import { ChevronDown, ChevronUp, Plus, Trash } from '@sim/emcn/icons'
 import { generateShortId } from '@sim/utils/id'
@@ -51,7 +51,10 @@ interface ViableModelOption {
 interface FallbackRowProps {
   row: FallbackModelEntry
   index: number
-  count: number
+  /** The row can move down only while another follows it. */
+  isLast: boolean
+  /** Move controls render only once a second row exists. */
+  canMove: boolean
   primaryModel: string
   primaryTuning: Partial<Record<FallbackTuningKnob, unknown>>
   viableOptions: ViableModelOption[]
@@ -69,7 +72,8 @@ interface FallbackRowProps {
 const FallbackRow = memo(function FallbackRow({
   row,
   index,
-  count,
+  isLast,
+  canMove,
   primaryModel,
   primaryTuning,
   viableOptions,
@@ -96,7 +100,12 @@ const FallbackRow = memo(function FallbackRow({
     return {
       needsApiKey: fallbackRowNeedsApiKey(row.model, primaryModel),
       tuningFields: getFallbackTuningKnobsToShow(row.model, primaryModel, primaryTuning).map(
-        (knob) => ({ knob, options: getTuningOptionsForModel(row.model, knob) ?? [] })
+        (knob) => ({
+          knob,
+          options: (getTuningOptionsForModel(row.model, knob) ?? []).map(
+            (value): ComboboxOption => ({ label: value, value })
+          ),
+        })
       ),
     }
   }, [row.model, primaryModel, primaryTuning])
@@ -112,7 +121,7 @@ const FallbackRow = memo(function FallbackRow({
       <div className='flex items-center justify-between rounded-t-[4px] border-[var(--border-1)] border-b bg-[var(--surface-4)] px-2.5 py-[5px]'>
         <span className='text-[var(--text-tertiary)] text-sm'>{ordinalChoiceLabel(index)}</span>
         <div className='flex items-center gap-2'>
-          {count > 1 && (
+          {canMove && (
             <>
               <Tooltip.Root>
                 <Tooltip.Trigger asChild>
@@ -133,7 +142,7 @@ const FallbackRow = memo(function FallbackRow({
                   <Button
                     variant='ghost'
                     onClick={() => onMove(row.id, 1)}
-                    disabled={readOnly || index === count - 1}
+                    disabled={readOnly || isLast}
                     className='h-auto p-0'
                     aria-label='Move down'
                   >
@@ -196,8 +205,8 @@ const FallbackRow = memo(function FallbackRow({
           <div key={knob} className='flex flex-col gap-1.5'>
             <Label>{FALLBACK_TUNING_LABELS[knob]}</Label>
             <Combobox
-              options={options.map((value) => ({ label: value, value }))}
-              value={row[knob] ?? options[0] ?? ''}
+              options={options}
+              value={row[knob] ?? options[0]?.value ?? ''}
               onChange={(value) => onChangeTuning(row.id, knob, value)}
               placeholder={`Select ${FALLBACK_TUNING_LABELS[knob].toLowerCase()}`}
               disabled={readOnly}
@@ -306,38 +315,53 @@ export function ModelFallbackList({
     return options
   }, [workspaceId, workspaceEnv, personalEnv, navigateToSettings])
 
+  /**
+   * Handlers read the latest rows through a ref so their identity survives an
+   * edit; otherwise every keystroke in one row would re-render all of them.
+   */
+  const rowsRef = useRef(rows)
+  useEffect(() => {
+    rowsRef.current = rows
+  }, [rows])
+
   const write = useCallback(
-    (next: FallbackModelEntry[]) => {
-      if (readOnly || next === rows) return
-      setStoreValue(next)
+    (transform: (current: FallbackModelEntry[]) => FallbackModelEntry[]) => {
+      if (readOnly) return
+      const current = rowsRef.current
+      const next = transform(current)
+      if (next !== current) setStoreValue(next)
     },
-    [readOnly, rows, setStoreValue]
+    [readOnly, setStoreValue]
   )
 
-  const handleAdd = useCallback(() => write(addFallbackRow(rows, generateShortId())), [rows, write])
+  const handleAdd = useCallback(
+    () => write((current) => addFallbackRow(current, generateShortId())),
+    [write]
+  )
   const handleRemove = useCallback(
-    (id: string) => write(removeFallbackRow(rows, id)),
-    [rows, write]
+    (id: string) => write((current) => removeFallbackRow(current, id)),
+    [write]
   )
   const handleMove = useCallback(
-    (id: string, direction: -1 | 1) => write(moveFallbackRow(rows, id, direction)),
-    [rows, write]
+    (id: string, direction: -1 | 1) => write((current) => moveFallbackRow(current, id, direction)),
+    [write]
   )
   const handleChangeModel = useCallback(
-    (id: string, model: string) => write(changeFallbackRowModel(rows, id, model, primaryModel)),
-    [rows, primaryModel, write]
+    (id: string, model: string) =>
+      write((current) => changeFallbackRowModel(current, id, model, primaryModel)),
+    [primaryModel, write]
   )
   const handleChangeTuning = useCallback(
     (id: string, knob: FallbackTuningKnob, value: string) =>
-      write(changeFallbackRowTuning(rows, id, knob, value)),
-    [rows, write]
+      write((current) => changeFallbackRowTuning(current, id, knob, value)),
+    [write]
   )
   const handleChangeApiKey = useCallback(
     (id: string, apiKey: string) => {
       if (apiKey === CREATE_SECRET_VALUE) return
-      write(changeFallbackRowApiKey(rows, id, apiKey))
+      write((current) => changeFallbackRowApiKey(current, id, apiKey))
     },
-    [rows, write]
+    [write]
   )
 
   return (
@@ -347,7 +371,8 @@ export function ModelFallbackList({
           key={row.id}
           row={row}
           index={index}
-          count={rows.length}
+          isLast={index === rows.length - 1}
+          canMove={rows.length > 1}
           primaryModel={primaryModel}
           primaryTuning={primaryTuning}
           viableOptions={viableOptions}

apps/sim/blocks/utils.test.ts

@@ -75,6 +75,7 @@ import {
   parseOptionalBooleanInput,
   parseOptionalJsonInput,
   parseOptionalNumberInput,
+  providerRequiresFamilyCredentials,
   requiresProviderFamilyCredentials,
 } from '@/blocks/utils'
 import { getProviderFromModel } from '@/providers/utils'
@@ -98,6 +99,15 @@ const BASE_CLOUD_MODELS: Record<string, string> = {
   'mistral-large-latest': 'mistral',
 }
 
+describe('providerRequiresFamilyCredentials', () => {
+  it('answers for a provider the caller already resolved', () => {
+    expect(providerRequiresFamilyCredentials('vertex')).toBe(true)
+    expect(providerRequiresFamilyCredentials('openai')).toBe(false)
+    expect(providerRequiresFamilyCredentials(null)).toBe(false)
+    expect(providerRequiresFamilyCredentials(undefined)).toBe(false)
+  })
+})
+
 describe('requiresProviderFamilyCredentials', () => {
   beforeEach(() => {
     setEnvFlags({ isHosted: false, isAzureConfigured: false, isOllamaConfigured: false })

apps/sim/blocks/utils.ts

@@ -292,7 +292,15 @@ export function getCohereRerankerApiKeyCondition() {
  * that provider family, so nothing outside the family can inherit them.
  */
 export function requiresProviderFamilyCredentials(model: string): boolean {
-  const provider = findProviderFromModel(model.trim())
+  return providerRequiresFamilyCredentials(findProviderFromModel(model.trim()))
+}
+
+/**
+ * The provider-keyed half of {@link requiresProviderFamilyCredentials}, for a
+ * caller that has already resolved the provider and must not pay for a second
+ * catalog scan.
+ */
+export function providerRequiresFamilyCredentials(provider: string | null | undefined): boolean {
   if (provider === 'vertex') return true
   if (provider === 'bedrock') return !isTruthy(getEnv('NEXT_PUBLIC_BEDROCK_DEFAULT_CREDENTIALS'))
   if (provider === 'azure-openai' || provider === 'azure-anthropic') {

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

@@ -119,6 +119,7 @@ describe('BlockExecutor retry', () => {
 
     await expect(executor.execute(createContext(state), createNode(block), block)).rejects.toThrow()
     expect(execute).toHaveBeenCalledTimes(1)
+    expect(execute.mock.calls[0][3]).not.toHaveProperty('retry')
   })
 
   it('tells each try where it sits in the policy, and a block without one nothing', async () => {
@@ -151,6 +152,29 @@ describe('BlockExecutor retry', () => {
     expect(executePlain.mock.calls[0][3]).not.toHaveProperty('retry')
   })
 
+  it('hands the same try position to a handler that takes the node', async () => {
+    const block = createBlock({ enabled: true, maxTries: 2, waitBetweenTriesMs: 0 })
+    const executeWithNode = vi
+      .fn()
+      .mockRejectedValueOnce(new Error('one'))
+      .mockResolvedValueOnce({ ok: true })
+    const execute = vi.fn()
+    const state = new ExecutionState()
+    const executor = buildExecutor(
+      block,
+      { canHandle: () => true, execute, executeWithNode },
+      state
+    )
+
+    await executor.execute(createContext(state), createNode(block), block)
+
+    expect(execute).not.toHaveBeenCalled()
+    expect(executeWithNode.mock.calls.map(([, , , metadata]) => metadata.retry)).toEqual([
+      { attempt: 1, maxTries: 2, isFinalTry: false },
+      { attempt: 2, maxTries: 2, isFinalTry: true },
+    ])
+  })
+
   it('replays any failure and succeeds on a later try', async () => {
     const block = createBlock(enabled)
     const execute = vi

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

@@ -569,12 +569,9 @@ export class BlockExecutor {
     try {
       for (;;) {
         tries++
+        const isFinalTry = tries >= policy.maxTries
         try {
-          const output = await invoke({
-            attempt: tries,
-            maxTries: policy.maxTries,
-            isFinalTry: tries >= policy.maxTries,
-          })
+          const output = await invoke({ attempt: tries, maxTries: policy.maxTries, isFinalTry })
           if (!shouldAccumulateFunctionCost || !accumulatedFunctionCost || !isRecordLike(output)) {
             return output
           }
@@ -596,7 +593,6 @@ export class BlockExecutor {
             )
           }
 
-          const isFinalTry = tries >= policy.maxTries
           if (isFinalTry || ctx.abortSignal?.aborted || !isRetryableBlockError(error)) {
             attachTrustedExecutionCost(error, accumulatedFunctionCost)
             throw error