Skip to content

fix(autofill-env-vars): simplify/remove logic to not cause useEffect overwrites #726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
icecrasher321 merged 2 commits into from
Jul 20, 2025
Merged

fix(autofill-env-vars): simplify/remove logic to not cause useEffect overwrites #726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81b2736
fix(autofill-env-vars): simplify logic to not cause useEffect overwrites
icecrasher321 Jul 19, 2025
b62317f
remove useless comments
icecrasher321 Jul 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions apps/sim/app/api/users/me/settings/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const logger = createLogger('UserSettingsAPI')
const SettingsSchema = z.object({
theme: z.enum(['system', 'light', 'dark']).optional(),
autoConnect: z.boolean().optional(),
autoFillEnvVars: z.boolean().optional(),
autoFillEnvVars: z.boolean().optional(), // DEPRECATED: kept for backwards compatibility
autoPan: z.boolean().optional(),
consoleExpandedByDefault: z.boolean().optional(),
telemetryEnabled: z.boolean().optional(),
Expand All @@ -31,7 +31,7 @@ const SettingsSchema = z.object({
const defaultSettings = {
theme: 'system',
autoConnect: true,
autoFillEnvVars: true,
autoFillEnvVars: true, // DEPRECATED: kept for backwards compatibility, always true
autoPan: true,
consoleExpandedByDefault: true,
telemetryEnabled: true,
Expand Down Expand Up @@ -65,7 +65,7 @@ export async function GET() {
data: {
theme: userSettings.theme,
autoConnect: userSettings.autoConnect,
autoFillEnvVars: userSettings.autoFillEnvVars,
autoFillEnvVars: userSettings.autoFillEnvVars, // DEPRECATED: kept for backwards compatibility
autoPan: userSettings.autoPan,
consoleExpandedByDefault: userSettings.consoleExpandedByDefault,
telemetryEnabled: userSettings.telemetryEnabled,
Expand Down
24 changes: 2 additions & 22 deletions ...owId]/components/workflow-block/components/sub-block/components/tool-input/tool-input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { cn } from '@/lib/utils'
import { getAllBlocks } from '@/blocks'
import { getProviderFromModel, supportsToolUsageControl } from '@/providers/utils'
import { useCustomToolsStore } from '@/stores/custom-tools/store'
import { useGeneralStore } from '@/stores/settings/general/store'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
import {
Expand Down Expand Up @@ -400,7 +399,6 @@ export function ToolInput({
const isWide = useWorkflowStore((state) => state.blocks[blockId]?.isWide)
const customTools = useCustomToolsStore((state) => state.getAllTools())
const subBlockStore = useSubBlockStore()
const isAutoFillEnvVarsEnabled = useGeneralStore((state) => state.isAutoFillEnvVarsEnabled)

// Get the current model from the 'model' subblock
const modelValue = useSubBlockStore.getState().getValue(blockId, 'model')
Expand Down Expand Up @@ -507,26 +505,13 @@ export function ToolInput({
return block.tools.access[0]
}

// Initialize tool parameters with auto-fill if enabled
// Initialize tool parameters - no autofill, just return empty params
const initializeToolParams = (
toolId: string,
params: ToolParameterConfig[],
instanceId?: string
): Record<string, string> => {
const initialParams: Record<string, string> = {}

// Only auto-fill parameters if the setting is enabled
if (isAutoFillEnvVarsEnabled) {
// For each parameter, check if we have a stored/resolved value
params.forEach((param) => {
const resolvedValue = subBlockStore.resolveToolParamValue(toolId, param.id, instanceId)
if (resolvedValue) {
initialParams[param.id] = resolvedValue
}
})
}

return initialParams
return {}
}

const handleSelectTool = (toolBlock: (typeof toolBlocks)[0]) => {
Expand Down Expand Up @@ -682,11 +667,6 @@ export function ToolInput({

const tool = selectedTools[toolIndex]

// Store the value in the tool params store for future use
if (paramValue.trim()) {
subBlockStore.setToolParam(tool.toolId, paramId, paramValue)
}

// Update the value in the workflow
setStoreValue(
selectedTools.map((tool, index) =>
Expand Down
238 changes: 23 additions & 215 deletions .../[workflowId]/components/workflow-block/components/sub-block/hooks/use-sub-block-value.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,161 +3,12 @@ import { isEqual } from 'lodash'
import { createLogger } from '@/lib/logs/console-logger'
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
import { getProviderFromModel } from '@/providers/utils'
import { useGeneralStore } from '@/stores/settings/general/store'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'

const logger = createLogger('SubBlockValue')

// Helper function to dispatch collaborative subblock updates
const dispatchSubblockUpdate = (blockId: string, subBlockId: string, value: any) => {
const event = new CustomEvent('update-subblock-value', {
detail: {
blockId,
subBlockId,
value,
},
})
window.dispatchEvent(event)
}

/**
* Helper to handle API key auto-fill for provider-based blocks
* Used for agent, router, evaluator, and any other blocks that use LLM providers
*/
function handleProviderBasedApiKey(
blockId: string,
subBlockId: string,
modelValue: string | null | undefined,
storeValue: any,
isModelChange = false
) {
// Only proceed if we have a model selected
if (!modelValue) return

// Get the provider for this model
const provider = getProviderFromModel(modelValue)

// Skip if we couldn't determine a provider
if (!provider || provider === 'ollama') return

const subBlockStore = useSubBlockStore.getState()
const isAutoFillEnabled = useGeneralStore.getState().isAutoFillEnvVarsEnabled

// Try to get a saved API key for this provider (only if auto-fill is enabled)
const savedValue = isAutoFillEnabled
? subBlockStore.resolveToolParamValue(provider, 'apiKey', blockId)
: null

// If we have a valid saved API key and auto-fill is enabled, use it
if (savedValue && savedValue !== '' && isAutoFillEnabled) {
// Only update if the current value is different to avoid unnecessary updates
if (storeValue !== savedValue) {
dispatchSubblockUpdate(blockId, subBlockId, savedValue)
}
} else if (isModelChange && (!storeValue || storeValue === '')) {
// Only clear the field when switching models AND the field is already empty
// Don't clear existing user-entered values on initial load
dispatchSubblockUpdate(blockId, subBlockId, '')
}
// If no saved value and this is initial load, preserve existing value
}

/**
* Helper to handle API key auto-fill for non-agent blocks
*/
function handleStandardBlockApiKey(
blockId: string,
subBlockId: string,
blockType: string | undefined,
storeValue: any
) {
if (!blockType) return

const subBlockStore = useSubBlockStore.getState()

// Only auto-fill if the field is empty
if (!storeValue || storeValue === '') {
// Pass the blockId as instanceId to check if this specific instance has been cleared
const savedValue = subBlockStore.resolveToolParamValue(blockType, 'apiKey', blockId)

if (savedValue && savedValue !== '' && savedValue !== storeValue) {
// Auto-fill the API key from the param store
dispatchSubblockUpdate(blockId, subBlockId, savedValue)
}
}
// Handle environment variable references
else if (
storeValue &&
typeof storeValue === 'string' &&
storeValue.startsWith('{{') &&
storeValue.endsWith('}}')
) {
// Pass the blockId as instanceId
const currentValue = subBlockStore.resolveToolParamValue(blockType, 'apiKey', blockId)

if (currentValue !== storeValue) {
// If we got a replacement or null, update the field
if (currentValue) {
// Replacement found - update to new reference
dispatchSubblockUpdate(blockId, subBlockId, currentValue)
}
}
}
}

/**
* Helper to store API key values
*/
function storeApiKeyValue(
blockId: string,
blockType: string | undefined,
modelValue: string | null | undefined,
newValue: any,
storeValue: any
) {
if (!blockType) return

const subBlockStore = useSubBlockStore.getState()

// Check if this is user explicitly clearing a field that had a value
// We only want to mark it as cleared if it's a user action, not an automatic
// clearing from model switching
if (
storeValue &&
storeValue !== '' &&
(newValue === null || newValue === '' || String(newValue).trim() === '')
) {
// Mark this specific instance as cleared so we don't auto-fill it
subBlockStore.markParamAsCleared(blockId, 'apiKey')
return
}

// Only store non-empty values
if (!newValue || String(newValue).trim() === '') return

// If user enters a value, we should clear any "cleared" flag
// to ensure auto-fill will work in the future
if (subBlockStore.isParamCleared(blockId, 'apiKey')) {
subBlockStore.unmarkParamAsCleared(blockId, 'apiKey')
}

// For provider-based blocks, store the API key under the provider name
if (
(blockType === 'agent' || blockType === 'router' || blockType === 'evaluator') &&
modelValue
) {
const provider = getProviderFromModel(modelValue)
if (provider && provider !== 'ollama') {
subBlockStore.setToolParam(provider, 'apiKey', String(newValue))
}
} else {
// For other blocks, store under the block type
subBlockStore.setToolParam(blockType, 'apiKey', String(newValue))
}
}

interface UseSubBlockValueOptions {
debounceMs?: number
isStreaming?: boolean // Explicit streaming state
Expand Down Expand Up @@ -199,9 +50,6 @@ export function useSubBlockValue<T = any>(
// Keep a ref to the latest value to prevent unnecessary re-renders
const valueRef = useRef<T | null>(null)

// Previous model reference for detecting model changes
const prevModelRef = useRef<string | null>(null)

// Streaming refs
const lastEmittedValueRef = useRef<T | null>(null)
const streamingValueRef = useRef<T | null>(null)
Expand All @@ -216,9 +64,6 @@ export function useSubBlockValue<T = any>(
const isApiKey =
subBlockId === 'apiKey' || (subBlockId?.toLowerCase().includes('apikey') ?? false)

// Check if auto-fill environment variables is enabled - always call this hook unconditionally
const isAutoFillEnvVarsEnabled = useGeneralStore((state) => state.isAutoFillEnvVarsEnabled)

// Always call this hook unconditionally - don't wrap it in a condition
const modelSubBlockValue = useSubBlockStore((state) =>
blockId ? state.getValue(blockId, 'model') : null
Expand Down Expand Up @@ -276,6 +121,29 @@ export function useSubBlockValue<T = any>(
},
}))

// Handle model changes for provider-based blocks - clear API key when provider changes
if (
subBlockId === 'model' &&
isProviderBasedBlock &&
newValue &&
typeof newValue === 'string'
) {
const currentApiKeyValue = useSubBlockStore.getState().getValue(blockId, 'apiKey')

// Only clear if there's currently an API key value
if (currentApiKeyValue && currentApiKeyValue !== '') {
const oldModelValue = storeValue as string
const oldProvider = oldModelValue ? getProviderFromModel(oldModelValue) : null
const newProvider = getProviderFromModel(newValue)

// Clear API key if provider changed
if (oldProvider !== newProvider) {
// Use collaborative function to clear the API key
collaborativeSetSubblockValue(blockId, 'apiKey', '')
}
}
}

// Ensure we're passing the actual value, not a reference that might change
const valueCopy =
newValue === null
Expand All @@ -284,11 +152,6 @@ export function useSubBlockValue<T = any>(
? JSON.parse(JSON.stringify(newValue))
: newValue

// Handle API key storage for reuse across blocks
if (isApiKey && blockType) {
storeApiKeyValue(blockId, blockType, modelValue, newValue, storeValue)
}

// If streaming, just store the value without emitting
if (isStreaming) {
streamingValueRef.current = valueCopy
Expand Down Expand Up @@ -320,61 +183,6 @@ export function useSubBlockValue<T = any>(
valueRef.current = storeValue !== undefined ? storeValue : initialValue
}, [])

// When component mounts, check for existing API key in toolParamsStore
useEffect(() => {
// Skip autofill if the feature is disabled in settings
if (!isAutoFillEnvVarsEnabled) return

// Only process API key fields
if (!isApiKey) return

// Handle different block types
if (isProviderBasedBlock) {
handleProviderBasedApiKey(blockId, subBlockId, modelValue, storeValue, false)
} else {
// Normal handling for non-provider blocks
handleStandardBlockApiKey(blockId, subBlockId, blockType, storeValue)
}
}, [
blockId,
subBlockId,
blockType,
storeValue,
isApiKey,
isAutoFillEnvVarsEnabled,
modelValue,
isProviderBasedBlock,
])

// Monitor for model changes in provider-based blocks
useEffect(() => {
// Only process API key fields in model-based blocks
if (!isApiKey || !isProviderBasedBlock) return

// Check if the model has changed
if (modelValue !== prevModelRef.current) {
// Update the previous model reference
prevModelRef.current = modelValue

// Handle API key auto-fill for model changes
if (modelValue) {
handleProviderBasedApiKey(blockId, subBlockId, modelValue, storeValue, true)
} else {
// If no model is selected, clear the API key field
dispatchSubblockUpdate(blockId, subBlockId, '')
}
}
}, [
blockId,
subBlockId,
blockType,
isApiKey,
modelValue,
isAutoFillEnvVarsEnabled,
storeValue,
isProviderBasedBlock,
])

// Update the ref if the store value changes
// This ensures we're always working with the latest value
useEffect(() => {
Expand Down
Loading