refactor: replace hand-rolled utilities and dead code with the shared forms (#7021)

* refactor: replace hand-rolled utilities and dead code with the shared forms

Each of these has a mandated helper or an established accessor in the repo that
the site predates or missed. All are behavior-preserving:

- `omit()` for the three `Object.fromEntries(Object.entries(x).filter(...))`
  block-input filters, which also recovers the `Omit<T, K>` typing that
  `Object.fromEntries` erases to an index signature.
- `getErrorMessage()` for the inline `instanceof Error` message ternary.
- `getBlock()` for two `getAllBlocks().find((b) => b.type === x)` scans, one of
  them inside a loop over selected tools. The same file already resolves the
  same values through `getBlock`.
- A memoised `Map` for three `.find()`-by-id scans over the workspace skill
  list, one of them inside a render `.map()`.
- `SELECTOR_SEARCH_STALE` for three copy-pasted `15 * 1000` literals. They are
  deliberately shorter than `SELECTOR_STALE`, so this is a new named constant
  rather than a fold into the existing one.
- Tailwind classes for the static half of two duplicated anchor styles, keeping
  only the genuinely dynamic `left`/`top` inline.
- Dropped the unused `catch` bindings on three intentional JSON-parse swallows.

`panel.tsx`'s run-button gate loses a `TODO`-stubbed `hasValidationErrors =
false` and the `isWorkflowBlocked` term built on it. That term was dead twice
over: it reduced to `isExecuting`, and the enclosing expression is already
guarded by `!isExecuting`.

* fix: guard the registry lookups, and scope the search-stale doc to its callers

`getBlock` normalizes its argument with `type.replace(...)`, so it throws on
`undefined` where the `getAllBlocks().find(...)` it replaced returned
`undefined` harmlessly. Both call sites can be reached without a type:
`tool-input` reads `state.blocks[blockId]?.type`, which is undefined once the
block is deleted while the panel is mounted — and `Record` indexing hides that
from the compiler, so it would have thrown during render. `agent-handler`'s
`tool.type` is optional and the compiler did catch it.

Also index the skill lookup in `resolveSkillsLabel`, which runs a `.find()`
inside a `.map()` for every block on the canvas — the case the memoised map in
`skill-input` addressed for one component while leaving the hot path.

`providers/utils.ts` keeps its `getAllBlocks().find(...)`: it takes the
registry as an injected dependency precisely so a client-reachable module never
imports it, and reaching for `getBlock` there would cross that boundary.

The new constant's doc claimed search-backed selectors take a shorter window.
Several still sit on `SELECTOR_STALE`, so it now describes the value its three
callers share rather than asserting a rule the tree does not follow.

* fix: guard the second registry lookup in tool-input

`selectedTools` validates only `value[0]?.type` and then casts the whole array,
so a persisted workflow whose later rows lost their `type` yields `undefined`
here — the cast is what makes the compiler believe otherwise. `getBlock`
normalizes with `type.replace`, so that throws during render.
This commit is contained in:
Waleed
2026-08-23 18:41:41 -07:00
committed by GitHub
parent fc7aa66a39
commit 49593b3191
17 changed files with 63 additions and 49 deletions
@@ -289,14 +289,8 @@ export const PlusMenuDropdown = React.memo(
<DropdownMenu open={open} onOpenChange={handleOpenChange}>
<DropdownMenuTrigger asChild>
<div
style={{
position: 'fixed',
left: anchorPos?.left ?? 0,
top: anchorPos?.top ?? 0,
width: 0,
height: 0,
pointerEvents: 'none',
}}
className='pointer-events-none fixed size-0'
style={{ left: anchorPos?.left ?? 0, top: anchorPos?.top ?? 0 }}
/>
</DropdownMenuTrigger>
<DropdownMenuContent
@@ -182,14 +182,8 @@ export const SkillsMenuDropdown = React.memo(
<DropdownMenu open={open} onOpenChange={handleOpenChange}>
<DropdownMenuTrigger asChild>
<div
style={{
position: 'fixed',
left: anchorPos?.left ?? 0,
top: anchorPos?.top ?? 0,
width: 0,
height: 0,
pointerEvents: 'none',
}}
className='pointer-events-none fixed size-0'
style={{ left: anchorPos?.left ?? 0, top: anchorPos?.top ?? 0 }}
/>
</DropdownMenuTrigger>
<DropdownMenuContent
@@ -44,12 +44,17 @@ export function SkillInput({
const [editingSkillId, setEditingSkillId] = useState<string | null>(null)
const [editingSkillSnapshot, setEditingSkillSnapshot] = useState<SkillDefinition | null>(null)
const skillsById = useMemo(
() => new Map(workspaceSkills.map((skill) => [skill.id, skill])),
[workspaceSkills]
)
// Prefer the live query cache so the modal reflects concurrent edits, but
// fall back to the click-time snapshot when a background refetch drops the
// skill — otherwise the modal would close mid-edit and silently discard the
// draft; saving surfaces the real server error instead.
const editingSkill = editingSkillId
? (workspaceSkills.find((s) => s.id === editingSkillId) ?? editingSkillSnapshot)
? (skillsById.get(editingSkillId) ?? editingSkillSnapshot)
: null
const selectedSkills: StoredSkill[] = useMemo(() => {
@@ -119,10 +124,10 @@ export function SkillInput({
const resolveSkillName = useCallback(
(stored: StoredSkill): string => {
const found = workspaceSkills.find((s) => s.id === stored.skillId)
const found = skillsById.get(stored.skillId)
return found?.name ?? stored.name ?? stored.skillId
},
[workspaceSkills]
[skillsById]
)
return (
@@ -141,7 +146,7 @@ export function SkillInput({
{selectedSkills.length > 0 &&
selectedSkills.map((stored, index) => {
const fullSkill = workspaceSkills.find((s) => s.id === stored.skillId)
const fullSkill = skillsById.get(stored.skillId)
const skillName = resolveSkillName(stored)
const workflowSearchHighlight = getWorkflowSearchLabelHighlight({
activeSearchTarget,
@@ -520,7 +520,7 @@ export const ToolInput = memo(function ToolInput({
// subBlock): shown in the picker but greyed out with a tooltip instead of added.
const blockType = useWorkflowStore(useCallback((state) => state.blocks[blockId]?.type, [blockId]))
const unsupportedToolTypes = useMemo<readonly ('mcp' | 'custom-tool')[]>(() => {
const block = getAllBlocks().find((b) => b.type === blockType)
const block = blockType ? getBlock(blockType) : undefined
return block?.subBlocks.find((sb) => sb.id === subBlockId)?.unsupportedToolTypes ?? []
}, [blockType, subBlockId])
const mcpUnsupported = unsupportedToolTypes.includes('mcp')
@@ -529,9 +529,8 @@ export const ToolInput = memo(function ToolInput({
// Look up credential type for reactive condition filtering (e.g. service account detection).
// Uses canonical resolution so the active field (basic vs advanced) is respected.
const toolCredentialId = useMemo(() => {
const allBlocks = getAllBlocks()
for (const [toolIndex, tool] of selectedTools.entries()) {
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
const blockConfig = tool.type ? getBlock(tool.type) : undefined
if (!blockConfig?.subBlocks) continue
// canonical-index-unscoped: a nested tool resolves against `tool.params`, which only ever
// holds action-surface values — a tool is never invoked in trigger mode.
@@ -640,13 +640,10 @@ export const Panel = memo(function Panel() {
setIsMenuOpen(false)
}, [collaborativeBatchToggleLocked])
// Compute run button state
const canRun = userPermissions.canRead // Running only requires read permissions
const canRun = userPermissions.canRead
const isLoadingPermissions = userPermissions.isLoading
const hasValidationErrors = false // TODO: Add validation logic if needed
const isWorkflowBlocked = isExecuting || hasValidationErrors
const isButtonDisabled =
!isExecuting && (isUsageGateLoading || isWorkflowBlocked || (!canRun && !isLoadingPermissions))
!isExecuting && (isUsageGateLoading || (!canRun && !isLoadingPermissions))
/**
* Register global keyboard shortcuts using the central commands registry.
+2 -3
View File
@@ -1,3 +1,4 @@
import { omit } from '@sim/utils/object'
import { FirefliesIcon } from '@/components/icons'
import { resolveHttpsUrlFromFileInput } from '@/lib/uploads/utils/file-utils'
import type { BlockConfig, BlockMeta } from '@/blocks/types'
@@ -698,9 +699,7 @@ Return ONLY the summary text - no quotes, no labels.`,
const firefliesV2SubBlocks = (FirefliesBlock.subBlocks || []).filter(
(subBlock) => subBlock.id !== 'audioUrl'
)
const firefliesV2Inputs = FirefliesBlock.inputs
? Object.fromEntries(Object.entries(FirefliesBlock.inputs).filter(([key]) => key !== 'audioUrl'))
: {}
const firefliesV2Inputs = FirefliesBlock.inputs ? omit(FirefliesBlock.inputs, ['audioUrl']) : {}
export const FirefliesV2Block: BlockConfig<FirefliesResponse> = {
...FirefliesBlock,
+2 -1
View File
@@ -1,3 +1,4 @@
import { omit } from '@sim/utils/object'
import { GrainIcon } from '@/components/icons'
import type { BlockConfig, BlockMeta } from '@/blocks/types'
import { AuthMode, IntegrationType } from '@/blocks/types'
@@ -758,7 +759,7 @@ export const GrainV2Block: BlockConfig = {
},
},
inputs: {
...Object.fromEntries(Object.entries(GrainBlock.inputs).filter(([key]) => key !== 'viewId')),
...omit(GrainBlock.inputs, ['viewId']),
apiKey: { type: 'string', description: 'Grain API key (Personal or Workspace Access Token)' },
hookType: { type: 'string', description: 'Grain event type for the webhook' },
hookInclude: {
+2 -3
View File
@@ -1,3 +1,4 @@
import { omit } from '@sim/utils/object'
import { STTIcon } from '@/components/icons'
import { AuthMode, type BlockConfig, IntegrationType } from '@/blocks/types'
import { createVersionedToolSelector, normalizeFileInput } from '@/blocks/utils'
@@ -368,9 +369,7 @@ export const SttBlock: BlockConfig<SttBlockResponse> = {
},
}
const sttV2Inputs = SttBlock.inputs
? Object.fromEntries(Object.entries(SttBlock.inputs).filter(([key]) => key !== 'audioUrl'))
: {}
const sttV2Inputs = SttBlock.inputs ? omit(SttBlock.inputs, ['audioUrl']) : {}
const sttV2SubBlocks = (SttBlock.subBlocks || []).filter((subBlock) => subBlock.id !== 'audioUrl')
export const SttV2Block: BlockConfig<SttBlockResponse> = {
@@ -32,7 +32,7 @@ import { selectModelBoundFileInputPaths } from '@/lib/uploads/utils/model-input'
import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server'
import { resolveCustomBlockToolBinding } from '@/lib/workflows/custom-blocks/operations'
import { getCustomToolById } from '@/lib/workflows/custom-tools/operations'
import { getAllBlocks } from '@/blocks'
import { getAllBlocks, getBlock } from '@/blocks'
import { assembleCustomBlockInputMapping, isCustomBlockType } from '@/blocks/custom/build-config'
import type { BlockOutput } from '@/blocks/types'
import { normalizeFileInput } from '@/blocks/utils'
@@ -857,7 +857,7 @@ export class AgentBlockHandler implements BlockHandler {
)
if (tool.type === 'mcp' || tool.type === 'custom-tool') return alignedParams
const blockInputs = getAllBlocks().find((block) => block.type === tool.type)?.inputs
const blockInputs = tool.type ? getBlock(tool.type)?.inputs : undefined
return prepareResolvedSecretProjectedInputs(alignedParams, blockInputs, formattedParams)
}
@@ -58,7 +58,7 @@ export class ApiBlockHandler implements BlockHandler {
if (trimmedBody.startsWith('{') || trimmedBody.startsWith('[')) {
processedInputs.body = JSON.parse(trimmedBody)
}
} catch (e) {}
} catch {}
} else if (processedInputs.body === null) {
processedInputs.body = undefined
}
+2 -2
View File
@@ -113,7 +113,7 @@ export class StreamingResponseFormatProcessor implements ResponseFormatStreamPro
return null
}
} catch (e) {}
} catch {}
const openBraces = (buffer.match(/\{/g) || []).length
const closeBraces = (buffer.match(/\}/g) || []).length
@@ -138,7 +138,7 @@ export class StreamingResponseFormatProcessor implements ResponseFormatStreamPro
return null
}
} catch (e) {}
} catch {}
}
return null
+2 -2
View File
@@ -1,3 +1,4 @@
import { getErrorMessage } from '@sim/utils/errors'
import { HttpError } from '@/lib/core/utils/http-error'
import type { ExecutionContext, ExecutionResult } from '@/executor/types'
import type { SerializedBlock } from '@/serializer/types'
@@ -49,8 +50,7 @@ export interface BlockExecutionErrorDetails {
* every block boundary.
*/
export function buildBlockExecutionError(details: BlockExecutionErrorDetails): Error {
const errorMessage =
details.error instanceof Error ? details.error.message : String(details.error)
const errorMessage = getErrorMessage(details.error)
const blockName = details.block.metadata?.name || details.block.id
const blockType = details.block.metadata?.id || 'unknown'
@@ -1,6 +1,10 @@
import { requestJson } from '@/lib/api/client/request'
import * as selectorContracts from '@/lib/api/contracts/selectors'
import { ensureCredential, SELECTOR_STALE } from '@/hooks/selectors/providers/shared'
import {
ensureCredential,
SELECTOR_SEARCH_STALE,
SELECTOR_STALE,
} from '@/hooks/selectors/providers/shared'
import type { SelectorDefinition, SelectorKey, SelectorQueryArgs } from '@/hooks/selectors/types'
export const googleSelectors = {
@@ -101,7 +105,7 @@ export const googleSelectors = {
selectorContracts.googleDriveFilesSelectorContract,
selectorContracts.googleDriveFileSelectorContract,
],
staleTime: 15 * 1000,
staleTime: SELECTOR_SEARCH_STALE,
getQueryKey: ({ context, search }: SelectorQueryArgs) => [
'selectors',
'google.drive',
@@ -1,7 +1,12 @@
import { requestJson } from '@/lib/api/client/request'
import * as selectorContracts from '@/lib/api/contracts/selectors'
import { fetchOAuthToken } from '@/hooks/selectors/helpers'
import { ensureCredential, ensureDomain, SELECTOR_STALE } from '@/hooks/selectors/providers/shared'
import {
ensureCredential,
ensureDomain,
SELECTOR_SEARCH_STALE,
SELECTOR_STALE,
} from '@/hooks/selectors/providers/shared'
import type { SelectorDefinition, SelectorKey, SelectorQueryArgs } from '@/hooks/selectors/types'
export const jiraSelectors = {
@@ -71,7 +76,7 @@ export const jiraSelectors = {
selectorContracts.jiraIssuesSelectorContract,
selectorContracts.jiraIssueSelectorContract,
],
staleTime: 15 * 1000,
staleTime: SELECTOR_SEARCH_STALE,
getQueryKey: ({ context, search }: SelectorQueryArgs) => [
'selectors',
'jira.issues',
@@ -2,6 +2,17 @@ import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types'
export const SELECTOR_STALE = 60 * 1000
/**
* The shorter stale window carried by `google.drive`, `jira.issues` and
* `webflow.items`, whose listings turn over faster than {@link SELECTOR_STALE}
* assumes.
*
* Not every search-backed selector uses it — several still sit on
* {@link SELECTOR_STALE} — so treat this as the value those three share rather
* than a rule about search.
*/
export const SELECTOR_SEARCH_STALE = 15 * 1000
export const ensureCredential = (context: SelectorContext, key: SelectorKey): string => {
if (!context.oauthCredential) {
throw new Error(`Missing credential for selector ${key}`)
@@ -1,6 +1,10 @@
import { requestJson } from '@/lib/api/client/request'
import * as selectorContracts from '@/lib/api/contracts/selectors'
import { ensureCredential, SELECTOR_STALE } from '@/hooks/selectors/providers/shared'
import {
ensureCredential,
SELECTOR_SEARCH_STALE,
SELECTOR_STALE,
} from '@/hooks/selectors/providers/shared'
import type { SelectorDefinition, SelectorKey, SelectorQueryArgs } from '@/hooks/selectors/types'
export const webflowSelectors = {
@@ -59,7 +63,7 @@ export const webflowSelectors = {
'webflow.items': {
key: 'webflow.items',
contracts: [selectorContracts.webflowItemsSelectorContract],
staleTime: 15 * 1000,
staleTime: SELECTOR_SEARCH_STALE,
getQueryKey: ({ context, search }: SelectorQueryArgs) => [
'selectors',
'webflow.items',
+3 -1
View File
@@ -556,13 +556,15 @@ export function resolveSkillsLabel(
if (subBlock?.type !== 'skill-input') return null
if (!Array.isArray(rawValue) || rawValue.length === 0) return null
const skillsById = new Map(skills.map((skill) => [skill.id, skill]))
const names = rawValue
.map((skill: unknown) => {
if (!skill || typeof skill !== 'object') return null
const s = skill as { skillId?: string; name?: string }
if (s.skillId) {
const found = skills.find((candidate) => candidate.id === s.skillId)
const found = skillsById.get(s.skillId)
if (found?.name) return found.name
}
if (typeof s.name === 'string' && s.name) return s.name