mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(custom-blocks): reserve system output names and hide own block from command modal (#5667)
* fix(custom-blocks): reserve system output names and hide own block from command modal * fix(mothership): constrain custom block image icons in subagent tool rows * improvement(custom-blocks): allow result as an exposed output name
This commit is contained in:
+7
-1
@@ -93,7 +93,13 @@ export function ToolCallItem({
|
||||
return (
|
||||
<div className='flex items-center gap-[6px] pl-6'>
|
||||
{BlockIcon && (
|
||||
<BlockIcon className='size-[14px] flex-shrink-0' style={getBareIconStyle(BlockIcon)} />
|
||||
// Size via inline style: a custom block's image icon carries a trailing
|
||||
// `size-full` that defeats size *classes* (it fills tiled surfaces), so a
|
||||
// class-only size renders the uploaded icon at natural size here.
|
||||
<BlockIcon
|
||||
className='size-[14px] flex-shrink-0'
|
||||
style={{ width: 14, height: 14, ...getBareIconStyle(BlockIcon) }}
|
||||
/>
|
||||
)}
|
||||
{isExecuting ? (
|
||||
<ShimmerText className='text-[13px] [--shimmer-rest:var(--text-secondary)]'>
|
||||
|
||||
+7
-4
@@ -97,6 +97,7 @@ export function SearchModal({
|
||||
const params = useParams()
|
||||
const router = useRouter()
|
||||
const workspaceId = params.workspaceId as string
|
||||
const currentWorkflowId = params.workflowId as string | undefined
|
||||
const inputRef = useRef<HTMLInputElement>(null)
|
||||
const [mounted, setMounted] = useState(false)
|
||||
const { navigateToSettings } = useSettingsNavigation()
|
||||
@@ -581,23 +582,25 @@ export function SearchModal({
|
||||
*/
|
||||
const filteredBlocks = useMemo(() => {
|
||||
if (!isOnWorkflowPage) return []
|
||||
// A custom block is hidden on its own source workflow's canvas — placing it
|
||||
// there recurses (same exclusion as the toolbar).
|
||||
return filterAndCap(
|
||||
blocks,
|
||||
blocks.filter((b) => !b.sourceWorkflowId || b.sourceWorkflowId !== currentWorkflowId),
|
||||
(b) => b.name,
|
||||
deferredSearch,
|
||||
(b) => b.searchValue
|
||||
)
|
||||
}, [isOnWorkflowPage, blocks, deferredSearch])
|
||||
}, [isOnWorkflowPage, blocks, deferredSearch, currentWorkflowId])
|
||||
|
||||
const filteredTools = useMemo(() => {
|
||||
if (!isOnWorkflowPage) return []
|
||||
return filterAndCap(
|
||||
tools,
|
||||
tools.filter((t) => !t.sourceWorkflowId || t.sourceWorkflowId !== currentWorkflowId),
|
||||
(t) => t.name,
|
||||
deferredSearch,
|
||||
(t) => t.searchValue
|
||||
)
|
||||
}, [isOnWorkflowPage, tools, deferredSearch])
|
||||
}, [isOnWorkflowPage, tools, deferredSearch, currentWorkflowId])
|
||||
|
||||
const filteredTriggers = useMemo(() => {
|
||||
if (!isOnWorkflowPage) return []
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
CUSTOM_BLOCK_TILE_COLOR,
|
||||
type CustomBlockRow,
|
||||
isCustomBlockType,
|
||||
isReservedOutputName,
|
||||
} from '@/blocks/custom/build-config'
|
||||
import type { BlockIcon } from '@/blocks/types'
|
||||
|
||||
@@ -33,6 +34,18 @@ describe('isCustomBlockType', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('isReservedOutputName', () => {
|
||||
it('rejects the system output fields case-insensitively', () => {
|
||||
expect(isReservedOutputName('cost')).toBe(true)
|
||||
expect(isReservedOutputName('Cost')).toBe(true)
|
||||
expect(isReservedOutputName(' success ')).toBe(true)
|
||||
expect(isReservedOutputName('error')).toBe(true)
|
||||
expect(isReservedOutputName('result')).toBe(false)
|
||||
expect(isReservedOutputName('cost_2')).toBe(false)
|
||||
expect(isReservedOutputName('summary')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildCustomBlockConfig', () => {
|
||||
const fields: WorkflowInputField[] = [
|
||||
{ name: 'title', type: 'string' },
|
||||
@@ -47,6 +60,7 @@ describe('buildCustomBlockConfig', () => {
|
||||
const config = buildCustomBlockConfig(row, fields, { icon })
|
||||
expect(config.type).toBe('custom_block_abc123')
|
||||
expect(config.name).toBe('Invoice Parser')
|
||||
expect(config.sourceWorkflowId).toBe('wf-1')
|
||||
expect(config.category).toBe('tools')
|
||||
expect(config.bgColor).toBe(CUSTOM_BLOCK_TILE_COLOR)
|
||||
expect(config.hideFromToolbar).toBeUndefined()
|
||||
|
||||
@@ -62,6 +62,20 @@ export const RESERVED_PARAMS = new Set([
|
||||
'advancedMode',
|
||||
])
|
||||
|
||||
/**
|
||||
* Output names the block projects itself (`success`/`error` from `buildOutputs`,
|
||||
* `cost` from the executor's billing aggregation). A user-named exposed output
|
||||
* must never shadow these — an output literally named `cost` would clobber the
|
||||
* billed cost. `result` is deliberately NOT reserved: it only exists as a system
|
||||
* field when no outputs are curated, which cannot co-occur with a named output.
|
||||
*/
|
||||
export const RESERVED_OUTPUT_NAMES = new Set(['success', 'error', 'cost'])
|
||||
|
||||
/** Whether an exposed-output name collides with a system output field. */
|
||||
export function isReservedOutputName(name: string): boolean {
|
||||
return RESERVED_OUTPUT_NAMES.has(name.trim().toLowerCase())
|
||||
}
|
||||
|
||||
/** Map a Start input field type to the editor sub-block type used to collect it. */
|
||||
function subBlockTypeForField(fieldType: string): SubBlockType {
|
||||
switch (fieldType) {
|
||||
@@ -122,6 +136,7 @@ export function buildCustomBlockConfig(
|
||||
type: row.type,
|
||||
name: row.name,
|
||||
description: row.description,
|
||||
sourceWorkflowId: row.workflowId,
|
||||
category: 'tools',
|
||||
longDescription:
|
||||
'A published workflow packaged as a reusable, self-contained block. Fill its input ' +
|
||||
|
||||
@@ -468,6 +468,12 @@ export interface BlockConfig<T extends ToolResponse = ToolResponse> {
|
||||
}
|
||||
}
|
||||
hideFromToolbar?: boolean
|
||||
/**
|
||||
* For published custom blocks only: the bound source workflow's id. Discovery
|
||||
* surfaces use it to hide a workflow's own block on that workflow's canvas
|
||||
* (placing it would recurse).
|
||||
*/
|
||||
sourceWorkflowId?: string
|
||||
/**
|
||||
* Marks an unreleased block. Preview blocks are hidden from every discovery
|
||||
* surface (toolbar, search, mentions, copilot/VFS, docs) in every environment —
|
||||
|
||||
@@ -32,7 +32,11 @@ import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/compo
|
||||
import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel'
|
||||
import { useProfilePictureUpload } from '@/app/workspace/[workspaceId]/settings/hooks/use-profile-picture-upload'
|
||||
import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard'
|
||||
import type { CustomBlockInput, CustomBlockOutput } from '@/blocks/custom/build-config'
|
||||
import {
|
||||
type CustomBlockInput,
|
||||
type CustomBlockOutput,
|
||||
isReservedOutputName,
|
||||
} from '@/blocks/custom/build-config'
|
||||
import { SettingRow } from '@/ee/components/setting-row'
|
||||
import {
|
||||
useCustomBlocks,
|
||||
@@ -56,12 +60,12 @@ const decodeOutput = (value: string) => {
|
||||
: { blockId: value.slice(0, i), path: value.slice(i + OUTPUT_SEP.length) }
|
||||
}
|
||||
|
||||
/** Derive a unique, friendly output name from a dot-path, avoiding collisions. */
|
||||
/** Derive a unique, friendly output name from a dot-path, avoiding collisions and reserved names. */
|
||||
function deriveOutputName(path: string, taken: Set<string>): string {
|
||||
const base = (path.split('.').pop() || path).replace(/[^a-zA-Z0-9_]/g, '_')
|
||||
let name = base
|
||||
let n = 2
|
||||
while (taken.has(name)) name = `${base}_${n++}`
|
||||
while (taken.has(name) || isReservedOutputName(name)) name = `${base}_${n++}`
|
||||
taken.add(name)
|
||||
return name
|
||||
}
|
||||
@@ -360,6 +364,11 @@ export function CustomBlockDetail({ blockId, workspaceId, onBack }: CustomBlockD
|
||||
setError('Output names must be unique')
|
||||
return
|
||||
}
|
||||
const reserved = exposedOutputs.find((o) => isReservedOutputName(o.name))
|
||||
if (reserved) {
|
||||
setError(`"${reserved.name}" is a reserved output name (success, error, cost)`)
|
||||
return
|
||||
}
|
||||
// Only the placeholder and required flag are authored; the field set/name/type
|
||||
// are always derived from the deployed Start. Persist only non-empty overrides.
|
||||
const inputPlaceholders = visibleInputs
|
||||
|
||||
@@ -518,6 +518,45 @@ describe('WorkflowBlockHandler', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('projectCustomBlockOutput', () => {
|
||||
const childResult = {
|
||||
success: true,
|
||||
output: { data: 'whole result' },
|
||||
logs: [{ blockId: 'b1', success: true, output: { data: { x: 42 }, price: 999 } }],
|
||||
}
|
||||
|
||||
it('maps each curated output to its named field plus system fields', () => {
|
||||
const result = (handler as any).projectCustomBlockOutput(
|
||||
childResult,
|
||||
[{ blockId: 'b1', path: 'data.x', name: 'answer' }],
|
||||
0.5
|
||||
)
|
||||
|
||||
expect(result).toEqual({ answer: 42, success: true, cost: { total: 0.5 } })
|
||||
})
|
||||
|
||||
it('never lets an exposed output named cost clobber the billed cost', () => {
|
||||
const result = (handler as any).projectCustomBlockOutput(
|
||||
childResult,
|
||||
[{ blockId: 'b1', path: 'price', name: 'cost' }],
|
||||
0.5
|
||||
)
|
||||
|
||||
expect(result.cost).toEqual({ total: 0.5 })
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it('exposes the whole child result when no outputs are curated', () => {
|
||||
const result = (handler as any).projectCustomBlockOutput(childResult, [], 0.5)
|
||||
|
||||
expect(result).toEqual({
|
||||
success: true,
|
||||
result: { data: 'whole result' },
|
||||
cost: { total: 0.5 },
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('remapCustomBlockInputKeys', () => {
|
||||
|
||||
@@ -888,14 +888,15 @@ export class WorkflowBlockHandler implements BlockHandler {
|
||||
return { success: true, result: executionResult.output ?? {}, ...cost }
|
||||
}
|
||||
const logs = executionResult.logs ?? []
|
||||
const output: Record<string, unknown> = { success: true, ...cost }
|
||||
const output: Record<string, unknown> = {}
|
||||
for (const { blockId, path, name } of exposedOutputs) {
|
||||
const log =
|
||||
[...logs].reverse().find((l) => l.blockId === blockId && l.success) ??
|
||||
[...logs].reverse().find((l) => l.blockId === blockId)
|
||||
output[name] = log ? getValueAtPath(log.output, path) : undefined
|
||||
}
|
||||
return output as BlockOutput
|
||||
// System fields spread last — pre-validation rows may still name an output cost/success.
|
||||
return { ...output, success: true, ...cost } as BlockOutput
|
||||
}
|
||||
|
||||
private mapChildOutputToParent(
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { z } from 'zod'
|
||||
import { workflowIdSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives'
|
||||
import { defineRouteContract } from '@/lib/api/contracts/types'
|
||||
import { isReservedOutputName } from '@/blocks/custom/build-config'
|
||||
|
||||
const inputFieldSchema = z.object({
|
||||
/** Stable per-field id — preserved so client block configs key sub-blocks on it
|
||||
@@ -36,6 +37,21 @@ const exposedOutputSchema = z.object({
|
||||
name: z.string().min(1).max(60),
|
||||
})
|
||||
|
||||
/**
|
||||
* Publish/update variant: rejects reserved system output names (`success`,
|
||||
* `error`, `cost`) that would shadow the block's own projected fields. The
|
||||
* read schema stays lenient so rows that predate this validation still parse.
|
||||
*/
|
||||
const exposedOutputWriteSchema = exposedOutputSchema.extend({
|
||||
name: z
|
||||
.string()
|
||||
.min(1)
|
||||
.max(60)
|
||||
.refine((name) => !isReservedOutputName(name), {
|
||||
message: 'Output name is reserved (success, error, cost)',
|
||||
}),
|
||||
})
|
||||
|
||||
export const customBlockSchema = z.object({
|
||||
id: z.string(),
|
||||
organizationId: z.string(),
|
||||
@@ -87,7 +103,7 @@ export const publishCustomBlockBodySchema = z.object({
|
||||
/** Per-input placeholder hints keyed by Start field id; the field set itself is always derived from the deployment. */
|
||||
inputs: z.array(inputPlaceholderSchema).max(50).optional(),
|
||||
/** Curated outputs; omit/empty to expose the child's whole result. */
|
||||
exposedOutputs: z.array(exposedOutputSchema).max(50).optional(),
|
||||
exposedOutputs: z.array(exposedOutputWriteSchema).max(50).optional(),
|
||||
})
|
||||
|
||||
export type PublishCustomBlockBody = z.input<typeof publishCustomBlockBodySchema>
|
||||
@@ -104,7 +120,7 @@ export const updateCustomBlockBodySchema = z
|
||||
/** A URL (https or internal serve path) sets/replaces the icon; `null` clears it (default icon). */
|
||||
iconUrl: iconUrlSchema.nullable().optional(),
|
||||
inputs: z.array(inputPlaceholderSchema).max(50).optional(),
|
||||
exposedOutputs: z.array(exposedOutputSchema).max(50).optional(),
|
||||
exposedOutputs: z.array(exposedOutputWriteSchema).max(50).optional(),
|
||||
})
|
||||
.refine((v) => Object.keys(v).length > 0, { message: 'At least one field is required' })
|
||||
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('@/lib/billing/core/subscription', () => ({
|
||||
isOrganizationOnEnterprisePlan: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/core/config/feature-flags', () => ({
|
||||
isFeatureEnabled: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/workflows/input-format', () => ({
|
||||
extractInputFieldsFromBlocks: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/workflows/persistence/utils', () => ({
|
||||
loadDeployedWorkflowState: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/workspaces/permissions/utils', () => ({
|
||||
getWorkspaceWithOwner: vi.fn(),
|
||||
}))
|
||||
|
||||
import {
|
||||
CustomBlockValidationError,
|
||||
publishCustomBlock,
|
||||
updateCustomBlock,
|
||||
} from '@/lib/workflows/custom-blocks/operations'
|
||||
|
||||
const publishParams = {
|
||||
organizationId: 'org-1',
|
||||
workspaceId: 'ws-1',
|
||||
workflowId: 'wf-1',
|
||||
userId: 'user-1',
|
||||
name: 'Enrich Lead',
|
||||
description: '',
|
||||
}
|
||||
|
||||
describe('reserved exposed-output names', () => {
|
||||
it('publishCustomBlock rejects an output named cost', async () => {
|
||||
await expect(
|
||||
publishCustomBlock({
|
||||
...publishParams,
|
||||
exposedOutputs: [{ blockId: 'b1', path: 'price', name: 'cost' }],
|
||||
})
|
||||
).rejects.toThrow(CustomBlockValidationError)
|
||||
})
|
||||
|
||||
it('publishCustomBlock rejects reserved names case-insensitively', async () => {
|
||||
await expect(
|
||||
publishCustomBlock({
|
||||
...publishParams,
|
||||
exposedOutputs: [{ blockId: 'b1', path: 'content', name: 'Success' }],
|
||||
})
|
||||
).rejects.toThrow('"Success" is a reserved output name (success, error, cost)')
|
||||
})
|
||||
|
||||
it('updateCustomBlock rejects a reserved output name', async () => {
|
||||
await expect(
|
||||
updateCustomBlock('cb-1', {
|
||||
exposedOutputs: [{ blockId: 'b1', path: 'content', name: 'error' }],
|
||||
})
|
||||
).rejects.toThrow(CustomBlockValidationError)
|
||||
})
|
||||
})
|
||||
@@ -15,7 +15,7 @@ import { extractInputFieldsFromBlocks, type WorkflowInputField } from '@/lib/wor
|
||||
import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils'
|
||||
import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils'
|
||||
import type { CustomBlockOutput, CustomBlockRow } from '@/blocks/custom/build-config'
|
||||
import { CUSTOM_BLOCK_TYPE_PREFIX } from '@/blocks/custom/build-config'
|
||||
import { CUSTOM_BLOCK_TYPE_PREFIX, isReservedOutputName } from '@/blocks/custom/build-config'
|
||||
|
||||
const logger = createLogger('CustomBlocksOperations')
|
||||
|
||||
@@ -362,6 +362,19 @@ export class CustomBlockValidationError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject exposed outputs whose name shadows a system output field. Authoritative
|
||||
* check: also covers callers that bypass the HTTP contract (copilot handler).
|
||||
*/
|
||||
function assertNoReservedOutputNames(exposedOutputs: CustomBlockOutput[] | undefined): void {
|
||||
const reserved = exposedOutputs?.find((o) => isReservedOutputName(o.name))
|
||||
if (reserved) {
|
||||
throw new CustomBlockValidationError(
|
||||
`"${reserved.name}" is a reserved output name (success, error, cost)`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Publish a deployed workflow as an org-wide custom block. The source workflow
|
||||
* must live in `workspaceId` — the workspace the caller was verified to admin —
|
||||
@@ -393,6 +406,8 @@ export async function publishCustomBlock(params: {
|
||||
exposedOutputs,
|
||||
} = params
|
||||
|
||||
assertNoReservedOutputNames(exposedOutputs)
|
||||
|
||||
const [wf] = await db
|
||||
.select({
|
||||
id: workflow.id,
|
||||
@@ -487,6 +502,7 @@ export async function updateCustomBlock(
|
||||
exposedOutputs?: CustomBlockOutput[]
|
||||
}
|
||||
): Promise<void> {
|
||||
if (updates.exposedOutputs !== undefined) assertNoReservedOutputNames(updates.exposedOutputs)
|
||||
const patch: Partial<typeof customBlock.$inferInsert> = { updatedAt: new Date() }
|
||||
if (updates.name !== undefined) patch.name = updates.name
|
||||
if (updates.description !== undefined) patch.description = updates.description
|
||||
|
||||
@@ -105,6 +105,7 @@ export const useSearchModalStore = create<SearchModalState>()(
|
||||
bgColor: block.bgColor || '#6B7280',
|
||||
type: block.type,
|
||||
searchValue: `${block.name} ${block.type} ${buildCommandSearchableOptionSearchValue(block)}`,
|
||||
sourceWorkflowId: block.sourceWorkflowId,
|
||||
}
|
||||
|
||||
if (block.category === 'blocks' && block.type !== 'starter') {
|
||||
|
||||
@@ -12,6 +12,8 @@ export interface SearchBlockItem {
|
||||
type: string
|
||||
config?: BlockConfig
|
||||
searchValue?: string
|
||||
/** Custom blocks only: bound source workflow id — hidden on that workflow's canvas. */
|
||||
sourceWorkflowId?: string
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user