mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(serializer): apply tools.config.params before validating required tool params (#4391)
* fix(serializer): apply tools.config.params before validating required tool params * fix(serializer): guard array results and drop redundant fallback in tool param validation * fix(blocks): align canonicalParamId with tool param name for file inputs Renames the canonical id from `document` to `file` on firecrawl, reducto v2, pulse v2, and extend v2 so pre-execution validation resolves the value under the same key the tool expects, eliminating false "missing required fields: file" errors at submit time. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(blocks): drop extraneous comments from canonical file-input renames Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(serializer): drop tools.config.params invocation; enforce canonical-id contract via audit The serializer's pre-execution validator no longer runs the block's `tools.config.params` mapper to discover renamed tool param ids. Instead it relies on the contract that every required+user-only tool param is backed by a subBlock whose `id` or `canonicalParamId` equals the tool param id, and a new audit (`bun run check:block-canonical`) enforces this. Migrates posthog (`personalApiKey` → canonical `apiKey`) so the audit passes cleanly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(scripts): consolidate block registry CI checks into one script Folds the canonical-id contract audit into the existing subblock ID stability script and renames it to `check-block-registry.ts`. Both checks share the same `getAllBlocks()` import and registry-invariant purpose, so a single CI gate now catches both regression classes. The early-exit path on the stability check no longer short-circuits the canonical-id check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(scripts): unify check-block-registry result reporting Each check returns a discriminated `CheckResult` (pass | skip | fail) so the runner prints one definitive line per check instead of mixing a "skipping" message with a redundant "passed" line. Failure messages include a per-check header explaining the runtime impact. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Waleed
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
9655e8e96c
commit
50e118a033
@@ -90,7 +90,7 @@ jobs:
|
||||
|
||||
echo "✅ All feature flags are properly configured"
|
||||
|
||||
- name: Check subblock ID stability
|
||||
- name: Check block registry invariants
|
||||
run: |
|
||||
if [ "${{ github.event_name }}" = "pull_request" ]; then
|
||||
BASE_REF="origin/${{ github.base_ref }}"
|
||||
@@ -98,7 +98,7 @@ jobs:
|
||||
else
|
||||
BASE_REF="HEAD~1"
|
||||
fi
|
||||
bun run apps/sim/scripts/check-subblock-id-stability.ts "$BASE_REF"
|
||||
bun run apps/sim/scripts/check-block-registry.ts "$BASE_REF"
|
||||
|
||||
- name: Lint code
|
||||
run: bun run lint:check
|
||||
|
||||
@@ -130,19 +130,25 @@ export const ExtendBlock: BlockConfig<ExtendParserOutput> = {
|
||||
},
|
||||
}
|
||||
|
||||
const extendV2Inputs = ExtendBlock.inputs
|
||||
const extendV2Inputs = {
|
||||
file: { type: 'json' as const, description: 'Document (file upload or file reference)' },
|
||||
apiKey: ExtendBlock.inputs?.apiKey,
|
||||
outputFormat: ExtendBlock.inputs?.outputFormat,
|
||||
chunking: ExtendBlock.inputs?.chunking,
|
||||
engine: ExtendBlock.inputs?.engine,
|
||||
}
|
||||
const extendV2SubBlocks = (ExtendBlock.subBlocks || []).flatMap((subBlock) => {
|
||||
if (subBlock.id === 'filePath') {
|
||||
return []
|
||||
}
|
||||
if (subBlock.id === 'fileUpload') {
|
||||
return [
|
||||
subBlock,
|
||||
{ ...subBlock, canonicalParamId: 'file' },
|
||||
{
|
||||
id: 'fileReference',
|
||||
title: 'Document',
|
||||
type: 'short-input' as SubBlockType,
|
||||
canonicalParamId: 'document',
|
||||
canonicalParamId: 'file',
|
||||
placeholder: 'Connect a file output from another block',
|
||||
mode: 'advanced' as const,
|
||||
required: true,
|
||||
@@ -173,7 +179,7 @@ export const ExtendV2Block: BlockConfig<ExtendParserOutput> = {
|
||||
apiKey: params.apiKey.trim(),
|
||||
}
|
||||
|
||||
const documentInput = normalizeFileInput(params.document, { single: true })
|
||||
const documentInput = normalizeFileInput(params.file, { single: true })
|
||||
if (!documentInput) {
|
||||
throw new Error('Document file is required')
|
||||
}
|
||||
|
||||
@@ -37,7 +37,7 @@ export const FirecrawlBlock: BlockConfig<FirecrawlResponse> = {
|
||||
id: 'fileUpload',
|
||||
title: 'Document',
|
||||
type: 'file-upload' as SubBlockType,
|
||||
canonicalParamId: 'document',
|
||||
canonicalParamId: 'file',
|
||||
acceptedTypes:
|
||||
'application/pdf,application/vnd.openxmlformats-officedocument.wordprocessingml.document,application/msword,application/vnd.oasis.opendocument.text,application/rtf,text/rtf,application/vnd.openxmlformats-officedocument.spreadsheetml.sheet,application/vnd.ms-excel,text/html',
|
||||
placeholder: 'Upload a document (PDF, DOCX, HTML, XLSX, etc.)',
|
||||
@@ -53,7 +53,7 @@ export const FirecrawlBlock: BlockConfig<FirecrawlResponse> = {
|
||||
id: 'fileReference',
|
||||
title: 'File Reference',
|
||||
type: 'short-input' as SubBlockType,
|
||||
canonicalParamId: 'document',
|
||||
canonicalParamId: 'file',
|
||||
placeholder: 'File reference from previous block',
|
||||
mode: 'advanced',
|
||||
condition: {
|
||||
@@ -487,7 +487,7 @@ Example 2 - Product Data:
|
||||
break
|
||||
|
||||
case 'parse': {
|
||||
const file = normalizeFileInput(params.document, { single: true })
|
||||
const file = normalizeFileInput(params.file, { single: true })
|
||||
if (!file) {
|
||||
throw new Error('A document file is required for the parse operation')
|
||||
}
|
||||
@@ -624,7 +624,7 @@ Example 2 - Product Data:
|
||||
},
|
||||
maxCredits: { type: 'number', description: 'Maximum credits to spend' },
|
||||
strictConstrainToURLs: { type: 'boolean', description: 'Limit agent to provided URLs only' },
|
||||
document: { type: 'json', description: 'Document input (file upload or file reference)' },
|
||||
file: { type: 'json', description: 'Document input (file upload or file reference)' },
|
||||
includeTags: { type: 'json', description: 'HTML tags to include during parsing' },
|
||||
excludeTags: { type: 'json', description: 'HTML tags to exclude during parsing' },
|
||||
parsers: { type: 'json', description: 'Parser configuration (e.g., [{"type": "pdf"}])' },
|
||||
|
||||
@@ -156,6 +156,7 @@ export const PostHogBlock: BlockConfig<PostHogResponse> = {
|
||||
id: 'personalApiKey',
|
||||
title: 'Personal API Key',
|
||||
type: 'short-input',
|
||||
canonicalParamId: 'apiKey',
|
||||
placeholder: 'Enter your PostHog personal API key',
|
||||
password: true,
|
||||
condition: {
|
||||
@@ -1192,9 +1193,6 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
|
||||
if (params.operation === 'posthog_get_project' && params.projectIdParam) {
|
||||
params.projectId = params.projectIdParam
|
||||
}
|
||||
if (params.personalApiKey) {
|
||||
params.apiKey = params.personalApiKey
|
||||
}
|
||||
|
||||
const flagOps = [
|
||||
'posthog_get_feature_flag',
|
||||
@@ -1276,7 +1274,7 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
|
||||
operation: { type: 'string', description: 'Operation to perform' },
|
||||
region: { type: 'string', description: 'PostHog region (us or eu)' },
|
||||
projectApiKey: { type: 'string', description: 'Project API key for public endpoints' },
|
||||
personalApiKey: { type: 'string', description: 'Personal API key for private endpoints' },
|
||||
apiKey: { type: 'string', description: 'Personal API key for private endpoints' },
|
||||
projectId: { type: 'string', description: 'PostHog project ID' },
|
||||
// Core Data
|
||||
event: { type: 'string', description: 'Event name' },
|
||||
|
||||
@@ -74,7 +74,6 @@ export const PulseBlock: BlockConfig<PulseParserOutput> = {
|
||||
apiKey: params.apiKey.trim(),
|
||||
}
|
||||
|
||||
// document is the canonical param from fileUpload (basic) or filePath (advanced)
|
||||
const documentInput = params.document
|
||||
if (typeof documentInput === 'object') {
|
||||
parameters.file = documentInput
|
||||
@@ -128,21 +127,25 @@ export const PulseBlock: BlockConfig<PulseParserOutput> = {
|
||||
},
|
||||
}
|
||||
|
||||
// PulseV2Block uses the same canonical param 'document' for both basic and advanced modes
|
||||
const pulseV2Inputs = PulseBlock.inputs
|
||||
const pulseV2Inputs = {
|
||||
file: { type: 'json' as const, description: 'Document (file upload or file reference)' },
|
||||
apiKey: PulseBlock.inputs?.apiKey,
|
||||
pages: PulseBlock.inputs?.pages,
|
||||
chunking: PulseBlock.inputs?.chunking,
|
||||
chunkSize: PulseBlock.inputs?.chunkSize,
|
||||
}
|
||||
const pulseV2SubBlocks = (PulseBlock.subBlocks || []).flatMap((subBlock) => {
|
||||
if (subBlock.id === 'filePath') {
|
||||
return [] // Remove the old filePath subblock
|
||||
return []
|
||||
}
|
||||
if (subBlock.id === 'fileUpload') {
|
||||
// Insert fileReference right after fileUpload
|
||||
return [
|
||||
subBlock,
|
||||
{ ...subBlock, canonicalParamId: 'file' },
|
||||
{
|
||||
id: 'fileReference',
|
||||
title: 'Document',
|
||||
type: 'short-input' as SubBlockType,
|
||||
canonicalParamId: 'document',
|
||||
canonicalParamId: 'file',
|
||||
placeholder: 'File reference',
|
||||
mode: 'advanced' as const,
|
||||
required: true,
|
||||
@@ -173,8 +176,7 @@ export const PulseV2Block: BlockConfig<PulseParserOutput> = {
|
||||
apiKey: params.apiKey.trim(),
|
||||
}
|
||||
|
||||
// document is the canonical param from fileUpload (basic) or fileReference (advanced)
|
||||
const normalizedFile = normalizeFileInput(params.document, { single: true })
|
||||
const normalizedFile = normalizeFileInput(params.file, { single: true })
|
||||
if (!normalizedFile) {
|
||||
throw new Error('Document file is required')
|
||||
}
|
||||
|
||||
@@ -71,7 +71,6 @@ export const ReductoBlock: BlockConfig<ReductoParserOutput> = {
|
||||
apiKey: params.apiKey.trim(),
|
||||
}
|
||||
|
||||
// document is the canonical param from fileUpload (basic) or filePath (advanced)
|
||||
const documentInput = params.document
|
||||
|
||||
if (typeof documentInput === 'object') {
|
||||
@@ -135,20 +134,24 @@ export const ReductoBlock: BlockConfig<ReductoParserOutput> = {
|
||||
},
|
||||
}
|
||||
|
||||
// ReductoV2Block uses the same canonical param 'document' for both basic and advanced modes
|
||||
const reductoV2Inputs = ReductoBlock.inputs
|
||||
const reductoV2Inputs = {
|
||||
file: { type: 'json' as const, description: 'PDF document (file upload or file reference)' },
|
||||
apiKey: ReductoBlock.inputs?.apiKey,
|
||||
pages: ReductoBlock.inputs?.pages,
|
||||
tableOutputFormat: ReductoBlock.inputs?.tableOutputFormat,
|
||||
}
|
||||
const reductoV2SubBlocks = (ReductoBlock.subBlocks || []).flatMap((subBlock) => {
|
||||
if (subBlock.id === 'filePath') {
|
||||
return []
|
||||
}
|
||||
if (subBlock.id === 'fileUpload') {
|
||||
return [
|
||||
subBlock,
|
||||
{ ...subBlock, canonicalParamId: 'file' },
|
||||
{
|
||||
id: 'fileReference',
|
||||
title: 'PDF Document',
|
||||
type: 'short-input' as SubBlockType,
|
||||
canonicalParamId: 'document',
|
||||
canonicalParamId: 'file',
|
||||
placeholder: 'File reference',
|
||||
mode: 'advanced' as const,
|
||||
required: true,
|
||||
@@ -178,12 +181,11 @@ export const ReductoV2Block: BlockConfig<ReductoParserOutput> = {
|
||||
apiKey: params.apiKey.trim(),
|
||||
}
|
||||
|
||||
// document is the canonical param from fileUpload (basic) or fileReference (advanced)
|
||||
const documentInput = normalizeFileInput(params.document, { single: true })
|
||||
if (!documentInput) {
|
||||
const fileInput = normalizeFileInput(params.file, { single: true })
|
||||
if (!fileInput) {
|
||||
throw new Error('PDF document file is required')
|
||||
}
|
||||
parameters.file = documentInput
|
||||
parameters.file = fileInput
|
||||
|
||||
let pagesArray: number[] | undefined
|
||||
if (params.pages && params.pages.trim() !== '') {
|
||||
|
||||
@@ -0,0 +1,265 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
/**
|
||||
* CI check: enforces block-registry invariants that protect the runtime.
|
||||
*
|
||||
* Two checks run in sequence:
|
||||
*
|
||||
* 1. **Subblock ID stability** — diffs the current registry against a base ref
|
||||
* and fails if any subblock ID was removed without a corresponding entry in
|
||||
* `SUBBLOCK_ID_MIGRATIONS`. Removing IDs without a migration breaks
|
||||
* deployed workflows.
|
||||
*
|
||||
* 2. **Canonical-id contract** — for every (block, tool) pair where the tool
|
||||
* param is `required: true` and `visibility: 'user-only'`, the block must
|
||||
* expose a subBlock whose `id` or `canonicalParamId` equals the tool param
|
||||
* id. The serializer's pre-execution validator depends on this contract to
|
||||
* resolve values via direct lookup; mismatches false-flag fields as missing
|
||||
* at submit time.
|
||||
*
|
||||
* Usage:
|
||||
* bun run apps/sim/scripts/check-block-registry.ts [base-ref]
|
||||
*
|
||||
* `base-ref` defaults to `HEAD~1`. In a PR CI pipeline, pass the merge base:
|
||||
* bun run apps/sim/scripts/check-block-registry.ts origin/main
|
||||
*/
|
||||
|
||||
import { execSync } from 'child_process'
|
||||
import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations'
|
||||
import { getAllBlocks } from '@/blocks/registry'
|
||||
import { tools as toolRegistry } from '@/tools/registry'
|
||||
|
||||
const baseRef = process.argv[2] || 'HEAD~1'
|
||||
|
||||
const gitRoot = execSync('git rev-parse --show-toplevel', { encoding: 'utf-8' }).trim()
|
||||
const gitOpts = { encoding: 'utf-8' as const, cwd: gitRoot }
|
||||
|
||||
type IdMap = Record<string, Set<string>>
|
||||
|
||||
/**
|
||||
* Extracts subblock IDs from the `subBlocks: [ ... ]` section of a block
|
||||
* definition. Only grabs the top-level `id:` of each subblock object —
|
||||
* ignores nested IDs inside `options`, `columns`, etc.
|
||||
*/
|
||||
function extractSubBlockIds(source: string): string[] {
|
||||
const startIdx = source.indexOf('subBlocks:')
|
||||
if (startIdx === -1) return []
|
||||
|
||||
const bracketStart = source.indexOf('[', startIdx)
|
||||
if (bracketStart === -1) return []
|
||||
|
||||
const ids: string[] = []
|
||||
let braceDepth = 0
|
||||
let bracketDepth = 0
|
||||
let i = bracketStart + 1
|
||||
bracketDepth = 1
|
||||
|
||||
while (i < source.length && bracketDepth > 0) {
|
||||
const ch = source[i]
|
||||
|
||||
if (ch === '[') bracketDepth++
|
||||
else if (ch === ']') {
|
||||
bracketDepth--
|
||||
if (bracketDepth === 0) break
|
||||
} else if (ch === '{') {
|
||||
braceDepth++
|
||||
if (braceDepth === 1) {
|
||||
const ahead = source.slice(i, i + 200)
|
||||
const idMatch = ahead.match(/{\s*(?:\/\/[^\n]*\n\s*)*id:\s*['"]([^'"]+)['"]/)
|
||||
if (idMatch) {
|
||||
ids.push(idMatch[1])
|
||||
}
|
||||
}
|
||||
} else if (ch === '}') {
|
||||
braceDepth--
|
||||
}
|
||||
|
||||
i++
|
||||
}
|
||||
|
||||
return ids
|
||||
}
|
||||
|
||||
function getCurrentIds(): IdMap {
|
||||
const map: IdMap = {}
|
||||
for (const block of getAllBlocks()) {
|
||||
map[block.type] = new Set(block.subBlocks.map((sb) => sb.id))
|
||||
}
|
||||
return map
|
||||
}
|
||||
|
||||
type PreviousIdsResult =
|
||||
| { kind: 'skip'; reason: string }
|
||||
| { kind: 'noop' }
|
||||
| { kind: 'ok'; map: IdMap }
|
||||
|
||||
function getPreviousIds(): PreviousIdsResult {
|
||||
const registryPath = 'apps/sim/blocks/registry.ts'
|
||||
const blocksDir = 'apps/sim/blocks/blocks'
|
||||
|
||||
let hasChanges = false
|
||||
try {
|
||||
const diff = execSync(
|
||||
`git diff --name-only ${baseRef} HEAD -- ${registryPath} ${blocksDir}`,
|
||||
gitOpts
|
||||
).trim()
|
||||
hasChanges = diff.length > 0
|
||||
} catch {
|
||||
return { kind: 'skip', reason: 'Could not diff against base ref' }
|
||||
}
|
||||
|
||||
if (!hasChanges) {
|
||||
return { kind: 'noop' }
|
||||
}
|
||||
|
||||
const map: IdMap = {}
|
||||
|
||||
try {
|
||||
const blockFiles = execSync(`git ls-tree -r --name-only ${baseRef} -- ${blocksDir}`, gitOpts)
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter((f) => f.endsWith('.ts') && !f.endsWith('.test.ts'))
|
||||
|
||||
for (const filePath of blockFiles) {
|
||||
let content: string
|
||||
try {
|
||||
content = execSync(`git show ${baseRef}:${filePath}`, gitOpts)
|
||||
} catch {
|
||||
continue
|
||||
}
|
||||
|
||||
const typeMatch = content.match(/BlockConfig\s*=\s*\{[\s\S]*?type:\s*['"]([^'"]+)['"]/)
|
||||
if (!typeMatch) continue
|
||||
const blockType = typeMatch[1]
|
||||
|
||||
const ids = extractSubBlockIds(content)
|
||||
if (ids.length === 0) continue
|
||||
|
||||
map[blockType] = new Set(ids)
|
||||
}
|
||||
} catch (err) {
|
||||
return { kind: 'skip', reason: `Could not read previous block files from ${baseRef}: ${err}` }
|
||||
}
|
||||
|
||||
return { kind: 'ok', map }
|
||||
}
|
||||
|
||||
type CheckResult =
|
||||
| { kind: 'pass'; message: string }
|
||||
| { kind: 'skip'; message: string }
|
||||
| { kind: 'fail'; errors: string[] }
|
||||
|
||||
function checkSubblockIdStability(): CheckResult {
|
||||
const previous = getPreviousIds()
|
||||
|
||||
if (previous.kind === 'skip') {
|
||||
return { kind: 'skip', message: `${previous.reason} — skipping subblock ID stability check` }
|
||||
}
|
||||
if (previous.kind === 'noop') {
|
||||
return {
|
||||
kind: 'skip',
|
||||
message: 'No block definition changes detected — skipping subblock ID stability check',
|
||||
}
|
||||
}
|
||||
|
||||
const current = getCurrentIds()
|
||||
const errors: string[] = []
|
||||
|
||||
for (const [blockType, prevIds] of Object.entries(previous.map)) {
|
||||
const currIds = current[blockType]
|
||||
if (!currIds) continue
|
||||
|
||||
const migrations = SUBBLOCK_ID_MIGRATIONS[blockType] ?? {}
|
||||
|
||||
for (const oldId of prevIds) {
|
||||
if (currIds.has(oldId)) continue
|
||||
if (oldId in migrations) continue
|
||||
|
||||
errors.push(
|
||||
`Block "${blockType}": subblock ID "${oldId}" was removed.\n` +
|
||||
` → Add a migration in SUBBLOCK_ID_MIGRATIONS (lib/workflows/migrations/subblock-migrations.ts)\n` +
|
||||
` mapping "${oldId}" to its replacement ID.`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if (errors.length === 0) {
|
||||
return { kind: 'pass', message: 'Subblock ID stability check passed' }
|
||||
}
|
||||
return { kind: 'fail', errors }
|
||||
}
|
||||
|
||||
function checkCanonicalIdContract(): CheckResult {
|
||||
const errors: string[] = []
|
||||
|
||||
for (const block of getAllBlocks()) {
|
||||
const access: string[] = block.tools?.access ?? []
|
||||
if (access.length === 0) continue
|
||||
|
||||
const subBlockKeys = new Set<string>()
|
||||
for (const sb of block.subBlocks ?? []) {
|
||||
if (sb.id) subBlockKeys.add(sb.id)
|
||||
const canonical = (sb as { canonicalParamId?: string }).canonicalParamId
|
||||
if (canonical) subBlockKeys.add(canonical)
|
||||
}
|
||||
|
||||
for (const toolId of access) {
|
||||
const tool = toolRegistry[toolId]
|
||||
if (!tool) continue
|
||||
|
||||
for (const [paramId, paramConfig] of Object.entries(tool.params ?? {})) {
|
||||
if (!paramConfig || typeof paramConfig !== 'object') continue
|
||||
const required = (paramConfig as { required?: boolean }).required === true
|
||||
const userOnly = (paramConfig as { visibility?: string }).visibility === 'user-only'
|
||||
if (!required || !userOnly) continue
|
||||
|
||||
if (!subBlockKeys.has(paramId)) {
|
||||
errors.push(
|
||||
`Block "${block.type}" → tool "${toolId}": required user-only param "${paramId}" has no subBlock with id or canonicalParamId === "${paramId}".\n` +
|
||||
' → Rename a subBlock id or canonicalParamId to match the tool param id,\n' +
|
||||
" and update the block's inputs + tools.config.params mapper to read from that key."
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (errors.length === 0) {
|
||||
return { kind: 'pass', message: 'Canonical-id contract check passed' }
|
||||
}
|
||||
return { kind: 'fail', errors }
|
||||
}
|
||||
|
||||
function reportResult(label: string, failureHeader: string, result: CheckResult): boolean {
|
||||
if (result.kind === 'pass') {
|
||||
console.log(`✓ ${result.message}`)
|
||||
return true
|
||||
}
|
||||
if (result.kind === 'skip') {
|
||||
console.log(`⚠ ${result.message}`)
|
||||
return true
|
||||
}
|
||||
console.error(`\n✗ ${label} FAILED\n`)
|
||||
if (failureHeader) console.error(`${failureHeader}\n`)
|
||||
for (const err of result.errors) {
|
||||
console.error(` ${err}\n`)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
const stabilityResult = checkSubblockIdStability()
|
||||
const canonicalResult = checkCanonicalIdContract()
|
||||
|
||||
const stabilityOk = reportResult(
|
||||
'Subblock ID stability check',
|
||||
'Removing subblock IDs breaks deployed workflows.\nEither revert the rename or add a migration entry.',
|
||||
stabilityResult
|
||||
)
|
||||
|
||||
const canonicalOk = reportResult(
|
||||
'Canonical-id contract check',
|
||||
"Misaligned ids cause the serializer's pre-execution validator to false-flag fields as missing at submit time.",
|
||||
canonicalResult
|
||||
)
|
||||
|
||||
process.exit(stabilityOk && canonicalOk ? 0 : 1)
|
||||
@@ -1,170 +0,0 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
/**
|
||||
* CI check: detect subblock ID renames that would break deployed workflows.
|
||||
*
|
||||
* Compares the current block registry against the parent commit.
|
||||
* If any subblock ID was removed from a block, it must have a corresponding
|
||||
* entry in SUBBLOCK_ID_MIGRATIONS — otherwise this script exits non-zero.
|
||||
*
|
||||
* Usage:
|
||||
* bun run apps/sim/scripts/check-subblock-id-stability.ts [base-ref]
|
||||
*
|
||||
* base-ref defaults to HEAD~1. In a PR CI pipeline, pass the merge base:
|
||||
* bun run apps/sim/scripts/check-subblock-id-stability.ts origin/main
|
||||
*/
|
||||
|
||||
import { execSync } from 'child_process'
|
||||
import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations'
|
||||
import { getAllBlocks } from '@/blocks/registry'
|
||||
|
||||
const baseRef = process.argv[2] || 'HEAD~1'
|
||||
|
||||
const gitRoot = execSync('git rev-parse --show-toplevel', { encoding: 'utf-8' }).trim()
|
||||
const gitOpts = { encoding: 'utf-8' as const, cwd: gitRoot }
|
||||
|
||||
type IdMap = Record<string, Set<string>>
|
||||
|
||||
/**
|
||||
* Extracts subblock IDs from the `subBlocks: [ ... ]` section of a block
|
||||
* definition. Only grabs the top-level `id:` of each subblock object —
|
||||
* ignores nested IDs inside `options`, `columns`, etc.
|
||||
*/
|
||||
function extractSubBlockIds(source: string): string[] {
|
||||
const startIdx = source.indexOf('subBlocks:')
|
||||
if (startIdx === -1) return []
|
||||
|
||||
const bracketStart = source.indexOf('[', startIdx)
|
||||
if (bracketStart === -1) return []
|
||||
|
||||
const ids: string[] = []
|
||||
let braceDepth = 0
|
||||
let bracketDepth = 0
|
||||
let i = bracketStart + 1
|
||||
bracketDepth = 1
|
||||
|
||||
while (i < source.length && bracketDepth > 0) {
|
||||
const ch = source[i]
|
||||
|
||||
if (ch === '[') bracketDepth++
|
||||
else if (ch === ']') {
|
||||
bracketDepth--
|
||||
if (bracketDepth === 0) break
|
||||
} else if (ch === '{') {
|
||||
braceDepth++
|
||||
if (braceDepth === 1) {
|
||||
const ahead = source.slice(i, i + 200)
|
||||
const idMatch = ahead.match(/{\s*(?:\/\/[^\n]*\n\s*)*id:\s*['"]([^'"]+)['"]/)
|
||||
if (idMatch) {
|
||||
ids.push(idMatch[1])
|
||||
}
|
||||
}
|
||||
} else if (ch === '}') {
|
||||
braceDepth--
|
||||
}
|
||||
|
||||
i++
|
||||
}
|
||||
|
||||
return ids
|
||||
}
|
||||
|
||||
function getCurrentIds(): IdMap {
|
||||
const map: IdMap = {}
|
||||
for (const block of getAllBlocks()) {
|
||||
map[block.type] = new Set(block.subBlocks.map((sb) => sb.id))
|
||||
}
|
||||
return map
|
||||
}
|
||||
|
||||
function getPreviousIds(): IdMap {
|
||||
const registryPath = 'apps/sim/blocks/registry.ts'
|
||||
const blocksDir = 'apps/sim/blocks/blocks'
|
||||
|
||||
let hasChanges = false
|
||||
try {
|
||||
const diff = execSync(
|
||||
`git diff --name-only ${baseRef} HEAD -- ${registryPath} ${blocksDir}`,
|
||||
gitOpts
|
||||
).trim()
|
||||
hasChanges = diff.length > 0
|
||||
} catch {
|
||||
console.log('⚠ Could not diff against base ref — skipping check')
|
||||
process.exit(0)
|
||||
}
|
||||
|
||||
if (!hasChanges) {
|
||||
console.log('✓ No block definition changes detected — nothing to check')
|
||||
process.exit(0)
|
||||
}
|
||||
|
||||
const map: IdMap = {}
|
||||
|
||||
try {
|
||||
const blockFiles = execSync(`git ls-tree -r --name-only ${baseRef} -- ${blocksDir}`, gitOpts)
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter((f) => f.endsWith('.ts') && !f.endsWith('.test.ts'))
|
||||
|
||||
for (const filePath of blockFiles) {
|
||||
let content: string
|
||||
try {
|
||||
content = execSync(`git show ${baseRef}:${filePath}`, gitOpts)
|
||||
} catch {
|
||||
continue
|
||||
}
|
||||
|
||||
const typeMatch = content.match(/BlockConfig\s*=\s*\{[\s\S]*?type:\s*['"]([^'"]+)['"]/)
|
||||
if (!typeMatch) continue
|
||||
const blockType = typeMatch[1]
|
||||
|
||||
const ids = extractSubBlockIds(content)
|
||||
if (ids.length === 0) continue
|
||||
|
||||
map[blockType] = new Set(ids)
|
||||
}
|
||||
} catch (err) {
|
||||
console.log(`⚠ Could not read previous block files from ${baseRef} — skipping check`, err)
|
||||
process.exit(0)
|
||||
}
|
||||
|
||||
return map
|
||||
}
|
||||
|
||||
const previous = getPreviousIds()
|
||||
const current = getCurrentIds()
|
||||
const errors: string[] = []
|
||||
|
||||
for (const [blockType, prevIds] of Object.entries(previous)) {
|
||||
const currIds = current[blockType]
|
||||
if (!currIds) continue
|
||||
|
||||
const migrations = SUBBLOCK_ID_MIGRATIONS[blockType] ?? {}
|
||||
|
||||
for (const oldId of prevIds) {
|
||||
if (currIds.has(oldId)) continue
|
||||
|
||||
if (oldId in migrations) continue
|
||||
|
||||
errors.push(
|
||||
`Block "${blockType}": subblock ID "${oldId}" was removed.\n` +
|
||||
` → Add a migration in SUBBLOCK_ID_MIGRATIONS (lib/workflows/migrations/subblock-migrations.ts)\n` +
|
||||
` mapping "${oldId}" to its replacement ID.`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if (errors.length > 0) {
|
||||
console.error('✗ Subblock ID stability check FAILED\n')
|
||||
console.error(
|
||||
'Removing subblock IDs breaks deployed workflows.\n' +
|
||||
'Either revert the rename or add a migration entry.\n'
|
||||
)
|
||||
for (const err of errors) {
|
||||
console.error(` ${err}\n`)
|
||||
}
|
||||
process.exit(1)
|
||||
} else {
|
||||
console.log('✓ Subblock ID stability check passed')
|
||||
process.exit(0)
|
||||
}
|
||||
@@ -433,12 +433,18 @@ export class Serializer {
|
||||
const currentToolId = toolAccess?.length > 0 ? this.selectToolId(blockConfig, params) : null
|
||||
const currentTool = currentToolId ? getTool(currentToolId) : null
|
||||
|
||||
// Validate tool parameters (for blocks with tools)
|
||||
// Validate tool parameters (for blocks with tools).
|
||||
// Lookup contract: a tool param's value lives under its own paramId in `params`.
|
||||
// Block subBlocks must align via either `id === paramId` or `canonicalParamId === paramId`
|
||||
// (enforced by apps/sim/scripts/check-block-registry.ts), so this validator never has to invoke
|
||||
// the block's `tools.config.params` mapper.
|
||||
if (currentTool) {
|
||||
Object.entries(currentTool.params || {}).forEach(([paramId, paramConfig]) => {
|
||||
if (paramConfig.required && paramConfig.visibility === 'user-only') {
|
||||
const matchingConfigs =
|
||||
blockConfig.subBlocks?.filter((sb: any) => sb.id === paramId) || []
|
||||
blockConfig.subBlocks?.filter(
|
||||
(sb: any) => sb.id === paramId || sb.canonicalParamId === paramId
|
||||
) || []
|
||||
|
||||
let shouldValidateParam = true
|
||||
|
||||
@@ -492,10 +498,13 @@ export class Serializer {
|
||||
const validatedByTool = new Set(currentTool ? Object.keys(currentTool.params || {}) : [])
|
||||
|
||||
blockConfig.subBlocks?.forEach((subBlockConfig: SubBlockConfig) => {
|
||||
// Skip if already validated via tool params
|
||||
// Skip if already validated via tool params (either by id or canonical bridge)
|
||||
if (validatedByTool.has(subBlockConfig.id)) {
|
||||
return
|
||||
}
|
||||
if (subBlockConfig.canonicalParamId && validatedByTool.has(subBlockConfig.canonicalParamId)) {
|
||||
return
|
||||
}
|
||||
|
||||
// Check if subBlock is visible
|
||||
const isVisible = shouldSerializeSubBlock(
|
||||
|
||||
Reference in New Issue
Block a user