mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-30 19:32:46 +08:00
fix(selectors): respect trigger credentials in trigger mode (#6952)
* fix(selectors): respect trigger credentials in trigger mode * fix(selectors): isolate trigger selector context * fix(selectors): isolate action credential fallback * fix(selectors): scope fork reconfigs to trigger mode --------- Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local> Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
This commit is contained in:
co-authored by
Bill Leoutsakos
Vikhyath Mondreti
parent
c16883e9dc
commit
afccfe98a2
+19
-6
@@ -2,7 +2,10 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import { getErrorMessage } from '@sim/utils/errors'
|
||||
import { isEqual } from 'es-toolkit'
|
||||
import { useStoreWithEqualityFn } from 'zustand/traditional'
|
||||
import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context'
|
||||
import {
|
||||
buildSelectorContextFromBlock,
|
||||
getSelectorContextSubBlocks,
|
||||
} from '@/lib/workflows/subblocks/context'
|
||||
import { buildCanonicalIndex, resolveDependencyValue } from '@/lib/workflows/subblocks/visibility'
|
||||
import { getBlock } from '@/blocks/registry'
|
||||
import { getSelectorDefinition, loadAllSelectorOptions } from '@/hooks/selectors/registry'
|
||||
@@ -93,10 +96,6 @@ export function useFetchedOptions({
|
||||
const blockState = useWorkflowStore((state) => state.blocks[blockId])
|
||||
const blockConfig = blockState?.type ? getBlock(blockState.type) : null
|
||||
const canonicalModeOverrides = blockState?.data?.canonicalModes
|
||||
const canonicalIndex = useMemo(
|
||||
() => buildCanonicalIndex(blockConfig?.subBlocks || []),
|
||||
[blockConfig?.subBlocks]
|
||||
)
|
||||
|
||||
const dependencyValues = useStoreWithEqualityFn(
|
||||
useSubBlockStore,
|
||||
@@ -105,11 +104,24 @@ export function useFetchedOptions({
|
||||
if (dependsOnFields.length === 0 || !activeWorkflowId) return []
|
||||
const workflowValues = state.workflowValues[activeWorkflowId] || {}
|
||||
const blockValues = workflowValues[blockId] || {}
|
||||
const contextConfigs = getSelectorContextSubBlocks(
|
||||
blockConfig?.subBlocks || [],
|
||||
blockValues,
|
||||
blockState?.triggerMode
|
||||
)
|
||||
const canonicalIndex = buildCanonicalIndex(contextConfigs)
|
||||
return dependsOnFields.map((depKey) =>
|
||||
resolveDependencyValue(depKey, blockValues, canonicalIndex, canonicalModeOverrides)
|
||||
)
|
||||
},
|
||||
[dependsOnFields, activeWorkflowId, blockId, canonicalIndex, canonicalModeOverrides]
|
||||
[
|
||||
dependsOnFields,
|
||||
activeWorkflowId,
|
||||
blockId,
|
||||
blockConfig?.subBlocks,
|
||||
blockState?.triggerMode,
|
||||
canonicalModeOverrides,
|
||||
]
|
||||
),
|
||||
isEqual
|
||||
)
|
||||
@@ -131,6 +143,7 @@ export function useFetchedOptions({
|
||||
workflowId: activeWorkflowId ?? undefined,
|
||||
workspaceId: workspaceId ?? undefined,
|
||||
canonicalModes: block.data?.canonicalModes,
|
||||
triggerMode: block.triggerMode,
|
||||
})
|
||||
if (selectorExcludeSelf && activeWorkflowId) context.excludeWorkflowId = activeWorkflowId
|
||||
return context
|
||||
|
||||
@@ -131,6 +131,91 @@ describe('collectForkDependentReconfigs', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('anchors duplicate trigger selectors to the active trigger credential', () => {
|
||||
vi.mocked(getBlock).mockReturnValue(
|
||||
blockWith([
|
||||
{
|
||||
id: 'credential',
|
||||
title: 'Credential',
|
||||
type: 'oauth-input',
|
||||
canonicalParamId: 'oauthCredential',
|
||||
mode: 'basic',
|
||||
},
|
||||
{
|
||||
id: 'workspacePicker',
|
||||
title: 'Workspace',
|
||||
type: 'project-selector',
|
||||
canonicalParamId: 'workspaceSlug',
|
||||
selectorKey: 'bitbucket.workspaces',
|
||||
dependsOn: ['credential'],
|
||||
mode: 'basic',
|
||||
},
|
||||
{
|
||||
id: 'selectedTriggerId',
|
||||
title: 'Trigger Type',
|
||||
type: 'dropdown',
|
||||
mode: 'trigger',
|
||||
},
|
||||
{
|
||||
id: 'triggerCredentials',
|
||||
title: 'Trigger Credential',
|
||||
type: 'oauth-input',
|
||||
canonicalParamId: 'oauthCredential',
|
||||
mode: 'trigger',
|
||||
condition: { field: 'selectedTriggerId', value: 'bitbucket_push' },
|
||||
},
|
||||
{
|
||||
id: 'workspacePicker',
|
||||
title: 'Workspace',
|
||||
type: 'project-selector',
|
||||
canonicalParamId: 'workspaceSlug',
|
||||
selectorKey: 'bitbucket.workspaces',
|
||||
dependsOn: ['triggerCredentials'],
|
||||
mode: 'trigger',
|
||||
condition: { field: 'selectedTriggerId', value: 'bitbucket_push' },
|
||||
},
|
||||
{
|
||||
id: 'triggerCredentials',
|
||||
title: 'Trigger Credential',
|
||||
type: 'oauth-input',
|
||||
canonicalParamId: 'oauthCredential',
|
||||
mode: 'trigger',
|
||||
condition: { field: 'selectedTriggerId', value: 'bitbucket_pull_request_created' },
|
||||
},
|
||||
{
|
||||
id: 'workspacePicker',
|
||||
title: 'Workspace',
|
||||
type: 'project-selector',
|
||||
canonicalParamId: 'workspaceSlug',
|
||||
selectorKey: 'bitbucket.workspaces',
|
||||
dependsOn: ['triggerCredentials'],
|
||||
mode: 'trigger',
|
||||
condition: { field: 'selectedTriggerId', value: 'bitbucket_pull_request_created' },
|
||||
},
|
||||
])
|
||||
)
|
||||
const state = sourceState('bitbucket', {
|
||||
credential: { value: 'dormant-action-credential' },
|
||||
selectedTriggerId: { value: 'bitbucket_push' },
|
||||
triggerCredentials: { value: 'active-trigger-credential' },
|
||||
workspacePicker: { value: 'acme' },
|
||||
})
|
||||
state.blocks['block-1'].triggerMode = true
|
||||
|
||||
const result = collectForkDependentReconfigs(
|
||||
[replaceItem],
|
||||
new Map([['wf-src', state]]),
|
||||
resolve
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0]).toMatchObject({
|
||||
parentSourceId: 'active-trigger-credential',
|
||||
subBlockKey: 'workspacePicker',
|
||||
currentValue: 'acme',
|
||||
})
|
||||
})
|
||||
|
||||
it('skips an anchor whose canonical pair is in advanced (manual) mode - the value passes through', () => {
|
||||
vi.mocked(getBlock).mockReturnValue(
|
||||
blockWith([
|
||||
|
||||
@@ -4,6 +4,7 @@ import { coerceObjectArray } from '@/lib/workflows/persistence/remap-internal-id
|
||||
import { getToolInputParamConfigs } from '@/lib/workflows/search-replace/indexer'
|
||||
import {
|
||||
buildSelectorContextFromBlock,
|
||||
getSelectorContextSubBlocks,
|
||||
SELECTOR_CONTEXT_FIELDS,
|
||||
} from '@/lib/workflows/subblocks/context'
|
||||
import {
|
||||
@@ -75,6 +76,8 @@ interface EmitAnchoredParams {
|
||||
targetWorkflowId: string
|
||||
/** Canonical-mode overrides for resolving the active parent member (undefined -> value heuristic). */
|
||||
canonicalModes?: CanonicalModeOverrides
|
||||
/** Restricts a top-level trigger-mode block to its condition-visible trigger fields. */
|
||||
triggerMode?: boolean
|
||||
/** Memoized so the deterministic target block id is derived at most once per block. */
|
||||
resolveTargetBlockId: () => string
|
||||
/** Map a dependent's config id to its wire `subBlockKey` (identity, or nested `tools[i].id`). */
|
||||
@@ -117,6 +120,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
|
||||
blockName,
|
||||
targetWorkflowId,
|
||||
canonicalModes,
|
||||
triggerMode,
|
||||
resolveTargetBlockId,
|
||||
makeSubBlockKey,
|
||||
makeTitle,
|
||||
@@ -127,18 +131,20 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
|
||||
} = params
|
||||
const fullContext = buildSelectorContextFromBlock(contextBlockType, contextSubBlocks, {
|
||||
canonicalModes,
|
||||
triggerMode,
|
||||
})
|
||||
const canonicalIndex = buildCanonicalIndex(config.subBlocks)
|
||||
const gates = createCanonicalModeGates(config.subBlocks, values, canonicalModes)
|
||||
const configById = new Map(config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
|
||||
const scanSubBlocks = getSelectorContextSubBlocks(config.subBlocks, values, triggerMode)
|
||||
const canonicalIndex = buildCanonicalIndex(scanSubBlocks)
|
||||
const gates = createCanonicalModeGates(scanSubBlocks, values, canonicalModes)
|
||||
const configById = new Map(scanSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
|
||||
// Shared with `applyDependentOverrides`, so what the modal offers is exactly what the sync
|
||||
// can write back — the two encoded this rule separately once and drifted.
|
||||
const reconfigurableIds = reconfigurableDependentIds(config.subBlocks)
|
||||
const reconfigurableIds = reconfigurableDependentIds(scanSubBlocks)
|
||||
// A field could hang off two anchors (or be reachable via two paths); emit it once.
|
||||
const seen = new Set<string>()
|
||||
|
||||
for (const anchor of PARENT_ANCHORS) {
|
||||
for (const anchorCfg of config.subBlocks) {
|
||||
for (const anchorCfg of scanSubBlocks) {
|
||||
if (anchorCfg.type !== anchor.subBlockType || !anchorCfg.id) continue
|
||||
// An anchor whose canonical pair is in ADVANCED (manual) mode is skipped entirely: the
|
||||
// active value is the user-owned manual member's, which is verbatim by policy - a sync
|
||||
@@ -169,7 +175,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
|
||||
if (typeof value === 'string' && value) context[key] = value
|
||||
}
|
||||
|
||||
for (const clear of getTransitiveSubBlockDependents(config.subBlocks, [anchorCfg.id])) {
|
||||
for (const clear of getTransitiveSubBlockDependents(scanSubBlocks, [anchorCfg.id])) {
|
||||
const dependent = configById.get(clear.subBlockId)
|
||||
// A dependent is offered when the modal can actually render a control for it: a
|
||||
// registered selector, or a plain text field. Anything else is skipped and the
|
||||
@@ -327,6 +333,7 @@ export function collectForkDependentReconfigs(
|
||||
blockName: block.name,
|
||||
targetWorkflowId: item.targetWorkflowId,
|
||||
canonicalModes: block.data?.canonicalModes,
|
||||
triggerMode: block.triggerMode,
|
||||
resolveTargetBlockId: resolveBlockId,
|
||||
makeSubBlockKey: (id) => id,
|
||||
makeTitle: (dependent) => dependent.title ?? dependent.id ?? '',
|
||||
|
||||
@@ -89,6 +89,7 @@ export function useDynamicSubBlockOptionDisplayName({
|
||||
workflowId: activeWorkflowId ?? undefined,
|
||||
workspaceId,
|
||||
canonicalModes: block.data?.canonicalModes,
|
||||
triggerMode: block.triggerMode,
|
||||
})
|
||||
}, [block, liveValues, activeWorkflowId, workspaceId])
|
||||
|
||||
|
||||
@@ -179,6 +179,7 @@ function extractSelectorContext(
|
||||
workflowId,
|
||||
workspaceId,
|
||||
canonicalModes: block.data?.canonicalModes,
|
||||
triggerMode: block.triggerMode,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -7,8 +7,12 @@ vi.unmock('@/blocks/registry')
|
||||
|
||||
import * as blocksBarrel from '@/blocks'
|
||||
import { getAllBlocks, getBlock as getRealBlock } from '@/blocks/registry'
|
||||
import { buildSelectorContextFromBlock, SELECTOR_CONTEXT_FIELDS } from './context'
|
||||
import { buildCanonicalIndex, isCanonicalPair } from './visibility'
|
||||
import {
|
||||
buildSelectorContextFromBlock,
|
||||
getSelectorContextSubBlocks,
|
||||
SELECTOR_CONTEXT_FIELDS,
|
||||
} from './context'
|
||||
import { buildCanonicalIndex, isCanonicalPair, resolveDependencyValue } from './visibility'
|
||||
|
||||
/**
|
||||
* Under `isolate: false` the module under test may already be cached from an
|
||||
@@ -23,6 +27,10 @@ afterAll(() => {
|
||||
getBlockSpy.mockRestore()
|
||||
})
|
||||
|
||||
function subBlocksFromValues(values: Record<string, unknown>): Record<string, { value: unknown }> {
|
||||
return Object.fromEntries(Object.entries(values).map(([id, value]) => [id, { value }]))
|
||||
}
|
||||
|
||||
describe('buildSelectorContextFromBlock', () => {
|
||||
it('should extract knowledgeBaseId from knowledgeBaseSelector via canonical mapping', () => {
|
||||
const ctx = buildSelectorContextFromBlock('knowledge', {
|
||||
@@ -152,6 +160,67 @@ describe('buildSelectorContextFromBlock', () => {
|
||||
).toBe('advanced-team')
|
||||
})
|
||||
|
||||
it('preserves Gmail action credential resolution in basic and advanced modes', () => {
|
||||
const subBlocks = subBlocksFromValues({
|
||||
credential: 'action-basic',
|
||||
manualCredential: 'action-advanced',
|
||||
})
|
||||
|
||||
expect(buildSelectorContextFromBlock('gmail', subBlocks).oauthCredential).toBe('action-basic')
|
||||
expect(
|
||||
buildSelectorContextFromBlock('gmail', subBlocks, {
|
||||
canonicalModes: { oauthCredential: 'advanced' },
|
||||
}).oauthCredential
|
||||
).toBe('action-advanced')
|
||||
expect(
|
||||
buildSelectorContextFromBlock(
|
||||
'gmail',
|
||||
subBlocksFromValues({ credential: '', triggerCredentials: 'dormant-trigger' })
|
||||
).oauthCredential
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('uses trigger credentials with and without canonical metadata after action conversion', () => {
|
||||
const clickupValues = {
|
||||
selectedTriggerId: 'clickup_task_created',
|
||||
credential: 'dormant-action',
|
||||
triggerCredentials: 'active-trigger',
|
||||
}
|
||||
const cases = [
|
||||
{ blockType: 'clickup', values: clickupValues },
|
||||
{
|
||||
blockType: 'airtable',
|
||||
values: { credential: 'dormant-action', triggerCredentials: 'active-trigger' },
|
||||
},
|
||||
]
|
||||
|
||||
for (const { blockType, values } of cases) {
|
||||
expect(
|
||||
buildSelectorContextFromBlock(blockType, subBlocksFromValues(values), {
|
||||
triggerMode: true,
|
||||
}).oauthCredential
|
||||
).toBe('active-trigger')
|
||||
}
|
||||
|
||||
const clickupConfig = getRealBlock('clickup')
|
||||
const triggerCanonicalIndex = buildCanonicalIndex(
|
||||
getSelectorContextSubBlocks(clickupConfig?.subBlocks ?? [], clickupValues, true)
|
||||
)
|
||||
expect(resolveDependencyValue('triggerCredentials', clickupValues, triggerCanonicalIndex)).toBe(
|
||||
'active-trigger'
|
||||
)
|
||||
})
|
||||
|
||||
it('does not leak a dormant action credential when an unmapped trigger credential is blank', () => {
|
||||
const ctx = buildSelectorContextFromBlock(
|
||||
'airtable',
|
||||
subBlocksFromValues({ credential: 'dormant-action', triggerCredentials: '' }),
|
||||
{ triggerMode: true }
|
||||
)
|
||||
|
||||
expect(ctx.oauthCredential).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should ignore subblock keys not in SELECTOR_CONTEXT_FIELDS', () => {
|
||||
const ctx = buildSelectorContextFromBlock('knowledge', {
|
||||
operation: { id: 'operation', type: 'dropdown', value: 'search' },
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import { getBlock } from '@/blocks'
|
||||
import type { SubBlockConfig } from '@/blocks/types'
|
||||
import type { SelectorContext } from '@/hooks/selectors/types'
|
||||
import type { SubBlockState } from '@/stores/workflows/workflow/types'
|
||||
import {
|
||||
buildCanonicalIndex,
|
||||
buildSubBlockValues,
|
||||
type CanonicalModeOverrides,
|
||||
evaluateSubBlockCondition,
|
||||
resolveActiveCanonicalValue,
|
||||
} from './visibility'
|
||||
|
||||
@@ -55,10 +57,26 @@ export const SELECTOR_CONTEXT_FIELDS = new Set<keyof SelectorContext>([
|
||||
'password',
|
||||
])
|
||||
|
||||
/**
|
||||
* Selects the block fields allowed to contribute to selector context for the active mode.
|
||||
*/
|
||||
export function getSelectorContextSubBlocks(
|
||||
subBlocks: SubBlockConfig[],
|
||||
values: Record<string, unknown>,
|
||||
triggerMode?: boolean
|
||||
): SubBlockConfig[] {
|
||||
if (!triggerMode) return subBlocks
|
||||
return subBlocks.filter(
|
||||
(subBlock) =>
|
||||
(subBlock.mode === 'trigger' || subBlock.mode === 'trigger-advanced') &&
|
||||
evaluateSubBlockCondition(subBlock.condition, values)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds a SelectorContext from a block's subBlocks using the canonical index.
|
||||
*
|
||||
* Iterates all subblocks, resolves each through canonicalIdBySubBlockId to get
|
||||
* Iterates the active mode's subblocks, resolves each through canonicalIdBySubBlockId to get
|
||||
* the canonical key, then checks it against SELECTOR_CONTEXT_FIELDS.
|
||||
* This avoids hardcoding subblock IDs and automatically handles basic/advanced
|
||||
* renames.
|
||||
@@ -66,7 +84,12 @@ export const SELECTOR_CONTEXT_FIELDS = new Set<keyof SelectorContext>([
|
||||
export function buildSelectorContextFromBlock(
|
||||
blockType: string,
|
||||
subBlocks: Record<string, SubBlockState | { value?: unknown }>,
|
||||
opts?: { workflowId?: string; workspaceId?: string; canonicalModes?: CanonicalModeOverrides }
|
||||
opts?: {
|
||||
workflowId?: string
|
||||
workspaceId?: string
|
||||
canonicalModes?: CanonicalModeOverrides
|
||||
triggerMode?: boolean
|
||||
}
|
||||
): SelectorContext {
|
||||
const context: SelectorContext = {}
|
||||
if (opts?.workflowId) context.workflowId = opts.workflowId
|
||||
@@ -75,8 +98,16 @@ export function buildSelectorContextFromBlock(
|
||||
const blockConfig = getBlock(blockType)
|
||||
if (!blockConfig) return context
|
||||
|
||||
const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks)
|
||||
const values = buildSubBlockValues(subBlocks)
|
||||
const contextConfigs = getSelectorContextSubBlocks(
|
||||
blockConfig.subBlocks,
|
||||
values,
|
||||
opts?.triggerMode
|
||||
)
|
||||
const canonicalIndex = buildCanonicalIndex(contextConfigs)
|
||||
const contextSubBlockIds = opts?.triggerMode
|
||||
? new Set(contextConfigs.map((subBlock) => subBlock.id))
|
||||
: undefined
|
||||
const resolvedGroups = new Set<string>()
|
||||
|
||||
const setField = (key: string, value: unknown) => {
|
||||
@@ -89,6 +120,7 @@ export function buildSelectorContextFromBlock(
|
||||
}
|
||||
|
||||
for (const [subBlockId, subBlock] of Object.entries(subBlocks)) {
|
||||
if (contextSubBlockIds && !contextSubBlockIds.has(subBlockId)) continue
|
||||
const canonicalId = canonicalIndex.canonicalIdBySubBlockId[subBlockId]
|
||||
if (canonicalId) {
|
||||
// A canonical group resolves to its ACTIVE member only (no last-write-wins between a
|
||||
@@ -110,9 +142,9 @@ export function buildSelectorContextFromBlock(
|
||||
//
|
||||
// Only fills a gap: a block that does declare the canonical id has already set it above,
|
||||
// including the basic/advanced active-member resolution this loop cannot express.
|
||||
if (!context.oauthCredential) {
|
||||
if (!context.oauthCredential && !resolvedGroups.has('oauthCredential')) {
|
||||
for (const [subBlockId, subBlock] of Object.entries(subBlocks)) {
|
||||
if (blockConfig.subBlocks.find((cfg) => cfg.id === subBlockId)?.type !== 'oauth-input') {
|
||||
if (contextConfigs.find((cfg) => cfg.id === subBlockId)?.type !== 'oauth-input') {
|
||||
continue
|
||||
}
|
||||
const value = subBlock?.value
|
||||
|
||||
Reference in New Issue
Block a user