From 0e06cd98763e241c306c122c5f9c557b659b913b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 8 Oct 2026 08:43:10 -0700 Subject: [PATCH 1/3] fix(workflows): lint unquoted string references in JSON fields and surface lint on deploy --- apps/docs/openapi-v2-workflows.json | 2 +- apps/sim/lib/api/contracts/v2/workflows.ts | 2 +- .../apply-workflow-operations.test.ts | 1 + .../lib/workflows/application/deployments.ts | 51 +++++ .../application/workflow-deployments.test.ts | 90 ++++++++ .../editing/json-literal-refs.test.ts | 199 ++++++++++++++++++ .../lib/workflows/editing/lint-report.test.ts | 49 ++++- apps/sim/lib/workflows/editing/lint-report.ts | 11 +- apps/sim/lib/workflows/editing/lint.ts | 125 +++++++++-- 9 files changed, 511 insertions(+), 19 deletions(-) create mode 100644 apps/sim/lib/workflows/editing/json-literal-refs.test.ts diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index f32a7151820..7e10327a0bc 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -7048,7 +7048,7 @@ "required": ["blockId", "blockName", "blockType", "field", "value", "kind", "reason"], "additionalProperties": false }, - "description": "Credential, resource, tool, and skill references that do not resolve. These values are still persisted; they are reported, not dropped." + "description": "Credential, resource, tool, and skill references that do not resolve, and `block-output` references that will not work as written: a block that does not exist, an output field the block does not have (reason `unknown-field`), or a text output written unquoted in a JSON field (reason `unquoted-json-string`). These values are still persisted; they are reported, not dropped." }, "tableFieldIssues": { "type": "array", diff --git a/apps/sim/lib/api/contracts/v2/workflows.ts b/apps/sim/lib/api/contracts/v2/workflows.ts index 1b92a6d87b4..4fac3137132 100644 --- a/apps/sim/lib/api/contracts/v2/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/workflows.ts @@ -2917,7 +2917,7 @@ const v2WorkflowLintSchema = z }) ) .describe( - 'Credential, resource, tool, and skill references that do not resolve. These values are still persisted; they are reported, not dropped.' + 'Credential, resource, tool, and skill references that do not resolve, and `block-output` references that will not work as written: a block that does not exist, an output field the block does not have (reason `unknown-field`), or a text output written unquoted in a JSON field (reason `unquoted-json-string`). These values are still persisted; they are reported, not dropped.' ), tableFieldIssues: z .array( diff --git a/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts b/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts index 45a08186833..0584915c66b 100644 --- a/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts +++ b/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts @@ -80,6 +80,7 @@ vi.mock('@/lib/workflows/editing/validation', () => ({ vi.mock('@/lib/workflows/editing/lint', () => ({ collectWorkflowFieldIssues: () => [], collectDanglingBlockOutputReferences: () => [], + collectUnquotedJsonStringReferences: () => [], lintEditedWorkflowState: hoisted.lintGraph, })) vi.mock('@/lib/billing/core/subscription', () => billingSubscriptionMock) diff --git a/apps/sim/lib/workflows/application/deployments.ts b/apps/sim/lib/workflows/application/deployments.ts index 0b7ce9cd071..903214d2dae 100644 --- a/apps/sim/lib/workflows/application/deployments.ts +++ b/apps/sim/lib/workflows/application/deployments.ts @@ -1,15 +1,21 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { resolvePrincipalAttribution, toPrincipalActor } from '@sim/auth/principal' +import { createLogger } from '@sim/logger' import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/workflow' +import { getErrorMessage } from '@sim/utils/errors' import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { listLiveWorkflowMcpToolsForWorkflow } from '@/lib/mcp/queries' import { notifyWorkflowReverted } from '@/lib/realtime/notify' import { listDeployedWebhookUrls } from '@/lib/webhooks/deployed-urls' import { requireWorkflowExecutionUserId } from '@/lib/workflows/application/authorization' import { defineAuthorizedWorkflowUseCase } from '@/lib/workflows/application/authorized-workflow-use-case' +import type { ActiveWorkflowApplicationContext } from '@/lib/workflows/application/context' import { workflowOperations } from '@/lib/workflows/application/operations' import { resolvePrincipalWorkflowContext } from '@/lib/workflows/application/principal-scope' +import { withWorkflowBlockScope } from '@/lib/workflows/application/workflow-block-scope' import { checkNeedsRedeployment } from '@/lib/workflows/deployment-status' +import { formatWorkflowLintMessage, hasWorkflowLintIssues } from '@/lib/workflows/editing/lint' +import { buildWorkflowLintReport } from '@/lib/workflows/editing/lint-report' import { getWorkflowDeploymentSummary, performActivateVersion, @@ -19,9 +25,12 @@ import { } from '@/lib/workflows/orchestration' import { findPreviousDeploymentVersion, + loadWorkflowDeploymentVersionState, updateDeploymentVersionMetadata, } from '@/lib/workflows/persistence/utils' +const logger = createLogger('WorkflowDeployments') + export interface DeployWorkflowInput { workflowId: string assertedWorkspaceId?: string @@ -88,6 +97,42 @@ async function requireMutableWorkflow(workflowId: string): Promise { } } +/** + * The lint findings of the version a deploy just made live, as one warning. + * + * Deploy does not refuse on lint: findings are advisory, and some depend on the + * identity that runs the workflow. But a caller that deployed without linting + * would otherwise first learn of a block that cannot run from a failed live + * execution. Linting is best-effort and never fails the deploy that preceded it. + */ +async function deployedVersionLintWarning( + context: ActiveWorkflowApplicationContext, + deploymentVersionId: string | undefined, + subjectUserId: string +): Promise { + if (!deploymentVersionId) return undefined + try { + const report = await withWorkflowBlockScope(context, async () => + buildWorkflowLintReport( + await loadWorkflowDeploymentVersionState( + context.workflowId, + deploymentVersionId, + context.workspaceId + ), + { workflowId: context.workflowId, workspaceId: context.workspaceId, subjectUserId } + ) + ) + if (!hasWorkflowLintIssues(report)) return undefined + return `The deployed version has lint findings and may fail when it runs. ${formatWorkflowLintMessage(report)}` + } catch (error) { + logger.warn('Deployed version lint failed', { + workflowId: context.workflowId, + error: getErrorMessage(error), + }) + return undefined + } +} + export const deployWorkflow = defineAuthorizedWorkflowUseCase({ operation: workflowOperations.deploy, resolveContext: resolvePrincipalWorkflowContext, @@ -108,8 +153,14 @@ export const deployWorkflow = defineAuthorizedWorkflowUseCase({ idempotencyKey: input.idempotencyKey, }) if (!result.success) throwDeploymentFailure(result, 'Failed to deploy workflow') + const lintWarning = await deployedVersionLintWarning( + context, + result.deploymentVersionId, + attribution.attributedUserId + ) return { ...result, + warnings: lintWarning ? [...(result.warnings ?? []), lintWarning] : result.warnings, workflowId: context.workflowId, workspaceId: context.workspaceId, } diff --git a/apps/sim/lib/workflows/application/workflow-deployments.test.ts b/apps/sim/lib/workflows/application/workflow-deployments.test.ts index ee12090ae3b..be47485cc11 100644 --- a/apps/sim/lib/workflows/application/workflow-deployments.test.ts +++ b/apps/sim/lib/workflows/application/workflow-deployments.test.ts @@ -26,6 +26,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ listMcpTools: vi.fn(), + buildWorkflowLintReport: vi.fn(), })) vi.mock('@sim/audit', () => auditMock) @@ -46,6 +47,10 @@ vi.mock('@/lib/mcp/queries', () => ({ vi.mock('@/lib/workflows/deployment-status', () => workflowDeploymentStatusMock) +vi.mock('@/lib/workflows/editing/lint-report', () => ({ + buildWorkflowLintReport: mocks.buildWorkflowLintReport, +})) + import { activateWorkflowVersion, deployWorkflow, @@ -78,6 +83,19 @@ const context = { billedAccountUserId: 'billing-owner-1', } +const cleanLint = { + sources: [], + sinks: [], + orphanBlocks: [], + emptyOutgoingPorts: [], + invalidBranchPorts: [], + invalidConnectionTargets: [], + fieldIssues: [], + unresolvedReferences: [], + tableFieldIssues: [], + notes: [], +} + const adminPrincipals: Array<{ principal: Principal; actorUserId: string }> = [ { principal: createSessionPrincipal({ userId: 'session-user' }), @@ -110,6 +128,7 @@ describe('workflow deployment application use cases', () => { success: true, deployedAt: new Date('2026-08-08T00:00:00Z'), version: 4, + deploymentVersionId: 'version-4', activeDeployment: null, latestDeploymentAttempt: null, warnings: [], @@ -128,6 +147,11 @@ describe('workflow deployment application use cases', () => { version: 3, }) mockRevert.mockResolvedValue({ success: true, lastSaved: 12345 }) + workflowsPersistenceUtilsMockFns.mockLoadWorkflowDeploymentVersionState.mockResolvedValue({ + blocks: {}, + edges: [], + }) + mocks.buildWorkflowLintReport.mockResolvedValue(cleanLint) }) it.each(adminPrincipals)( @@ -297,4 +321,70 @@ describe('workflow deployment application use cases', () => { }) ).rejects.toThrow('Failed to deploy workflow') }) + + /** + * A draft whose Table block could never parse its row JSON deployed with an + * empty `warnings`, so the caller first learned of it from a failed live run. + */ + describe('lint findings on the deployed graph', () => { + const unquotedRowJson = { + ...cleanLint, + unresolvedReferences: [ + { + blockId: 'insert', + blockName: 'Insert Order', + field: 'data', + value: [''], + kind: 'block-output' as const, + reason: 'unquoted-json-string: quote it', + }, + ], + } + + it('reports them as a deploy warning, linted as the deploying user', async () => { + mockDeploy.mockResolvedValueOnce({ + success: true, + version: 4, + deploymentVersionId: 'version-4', + warnings: [ + 'Deployment activation completed, and post-activation notifications are queued.', + ], + }) + mocks.buildWorkflowLintReport.mockResolvedValueOnce(unquotedRowJson) + + const result = await deployWorkflow.execute({ + principal: createSessionPrincipal({ userId: 'session-user' }), + input: { workflowId: 'workflow-1', requestId: 'request-10' }, + }) + + expect(mocks.buildWorkflowLintReport).toHaveBeenCalledWith( + { blocks: {}, edges: [] }, + expect.objectContaining({ workflowId: 'workflow-1', subjectUserId: 'session-user' }) + ) + expect(result.warnings).toHaveLength(2) + expect(result.warnings?.[1]).toContain('"Insert Order".data ') + }) + + it('adds nothing for a clean graph', async () => { + const result = await deployWorkflow.execute({ + principal: createSessionPrincipal(), + input: { workflowId: 'workflow-1', requestId: 'request-11' }, + }) + + expect(result.warnings).toEqual([]) + }) + + it('never fails or blocks a deploy when lint cannot run', async () => { + workflowsPersistenceUtilsMockFns.mockLoadWorkflowDeploymentVersionState.mockRejectedValueOnce( + new Error('version read failed') + ) + + const result = await deployWorkflow.execute({ + principal: createSessionPrincipal(), + input: { workflowId: 'workflow-1', requestId: 'request-12' }, + }) + + expect(result).toMatchObject({ success: true, version: 4, warnings: [] }) + }) + }) }) diff --git a/apps/sim/lib/workflows/editing/json-literal-refs.test.ts b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts new file mode 100644 index 00000000000..3879d5bcf27 --- /dev/null +++ b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts @@ -0,0 +1,199 @@ +import type { Mock } from 'vitest' +import { describe, expect, it, vi } from 'vitest' +import { collectUnquotedJsonStringReferences } from '@/lib/workflows/editing/lint' +import { getAllBlocks, getBlock, getBlockByToolName, getBlockRegistry } from '@/blocks/registry' + +const mockGetBlock = getBlock as Mock +const mockGetAllBlocks = getAllBlocks as Mock +const mockGetBlockRegistry = getBlockRegistry as Mock +const mockGetBlockByToolName = getBlockByToolName as Mock +mockGetBlock.mockImplementation((type: string) => MOCK_BLOCKS[type]) +mockGetAllBlocks.mockImplementation(() => Object.values(MOCK_BLOCKS)) +mockGetBlockRegistry.mockImplementation(() => MOCK_BLOCKS) +mockGetBlockByToolName.mockImplementation(() => undefined) + +/** + * The few block shapes the check reads: which sub-blocks are JSON editors, and + * the declared output types references resolve to. + */ +const MOCK_BLOCKS = vi.hoisted( + () => + ({ + start_trigger: { + type: 'start_trigger', + category: 'triggers', + subBlocks: [{ id: 'inputFormat', type: 'input-format' }], + outputs: {}, + triggers: { enabled: true, available: ['chat', 'manual', 'api'] }, + }, + table_v2: { + type: 'table_v2', + category: 'blocks', + subBlocks: [ + { id: 'operation', type: 'dropdown' }, + { id: 'data', type: 'code', language: 'json' }, + { id: 'rows', type: 'code', language: 'json' }, + ], + outputs: {}, + }, + api: { + type: 'api', + category: 'blocks', + subBlocks: [ + { id: 'url', type: 'short-input' }, + { id: 'body', type: 'code', language: 'json' }, + ], + outputs: {}, + }, + function: { + type: 'function', + category: 'blocks', + subBlocks: [{ id: 'code', type: 'code', language: 'javascript' }], + outputs: { + result: { type: 'json', description: 'Return value' }, + stdout: { type: 'string', description: 'Console output' }, + }, + }, + agent: { + type: 'agent', + category: 'blocks', + subBlocks: [], + outputs: { + content: { type: 'string', description: 'Generated response content' }, + tokens: { type: 'json', description: 'Token usage' }, + }, + }, + }) as Record +) + +const START = { + type: 'start_trigger', + name: 'Start', + subBlocks: { + inputFormat: { + value: [ + { name: 'order_id', type: 'string' }, + { name: 'amount', type: 'number' }, + { name: 'paid', type: 'boolean' }, + { name: 'meta', type: 'object' }, + ], + }, + }, +} + +function graph( + blocks: Record }> +) { + return { blocks } as Parameters[0] +} + +function insertRow(data: unknown) { + return { type: 'table_v2', name: 'Insert Order', subBlocks: { data: { value: data } } } +} + +describe('collectUnquotedJsonStringReferences', () => { + it('flags a string input pasted unquoted into Table row JSON', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + insert: insertRow( + '{"order_id": , "amount": , "status": "pending"}' + ), + }) + ) + expect(findings).toHaveLength(1) + expect(findings[0]).toMatchObject({ + blockId: 'insert', + blockName: 'Insert Order', + field: 'data', + kind: 'block-output', + value: [''], + }) + expect(findings[0]?.reason).toMatch(/^unquoted-json-string: /) + expect(findings[0]?.reason).toContain('""') + }) + + it('flags every unquoted string reference in the field once, including agent text', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + writer: { type: 'agent', name: 'Writer' }, + call: { + type: 'api', + name: 'Post', + subBlocks: { + body: { + value: + '{"id": , "again": , "note": }', + }, + }, + }, + }) + ) + expect(findings).toHaveLength(1) + expect(findings[0]?.value).toEqual(['', '']) + }) + + it('accepts quoted string references, including inside escaped quotes', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + insert: insertRow( + '{"order_id": "", "label": "id \\"\\" ok", "tag": "a { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + build: { type: 'function', name: 'Build Row' }, + insert: insertRow( + '{"amount": , "paid": , "meta": , "row": }' + ), + many: { + type: 'table_v2', + name: 'Insert Many', + subBlocks: { rows: { value: '' } }, + }, + }) + ) + expect(findings).toHaveLength(0) + }) + + it('leaves references of unknown type and unresolvable heads to the other checks', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + build: { type: 'function', name: 'Build Row' }, + insert: insertRow( + '{"a": , "b": , "c": , "d": }' + ), + }) + ) + expect(findings).toHaveLength(0) + }) + + it('ignores fields that are not JSON editors and non-string values', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + call: { + type: 'api', + name: 'Post', + subBlocks: { url: { value: 'https://x.test/' } }, + }, + fn: { + type: 'function', + name: 'Fn', + subBlocks: { code: { value: 'return ' } }, + }, + insert: insertRow({ order_id: '' }), + }) + ) + expect(findings).toHaveLength(0) + }) +}) diff --git a/apps/sim/lib/workflows/editing/lint-report.test.ts b/apps/sim/lib/workflows/editing/lint-report.test.ts index 7284195e5ce..db87e259cfa 100644 --- a/apps/sim/lib/workflows/editing/lint-report.test.ts +++ b/apps/sim/lib/workflows/editing/lint-report.test.ts @@ -1,5 +1,6 @@ import { tableServiceMock, tableServiceMockFns } from '@sim/testing/mocks/table-service.mock' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Mock } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ collectUnresolvedReferences: vi.fn(async () => []), @@ -22,8 +23,10 @@ import { NO_ENTRY_BLOCK_NOTE, REFERENCES_UNCHECKED_NOTE, } from '@/lib/workflows/editing/lint-report' +import { getBlock } from '@/blocks/registry' const mockGetTableById = tableServiceMockFns.mockGetTableById +const mockGetBlock = getBlock as Mock const scope = { workflowId: 'workflow-1', workspaceId: 'workspace-1', subjectUserId: 'user-1' } @@ -218,3 +221,47 @@ describe('buildWorkflowLintReport table fields', () => { ) }) }) + +describe('buildWorkflowLintReport JSON fields', () => { + const registryStub = mockGetBlock.getMockImplementation() + + beforeEach(() => { + mockGetBlock.mockImplementation((type: string) => + type === 'table_v2' + ? { type, subBlocks: [{ id: 'data', type: 'code', language: 'json' }], outputs: {} } + : { type, category: 'triggers', subBlocks: [], outputs: {} } + ) + }) + + afterEach(() => { + mockGetBlock.mockImplementation(registryStub ?? (() => undefined)) + }) + + /** + * An unquoted string reference in Table row JSON reported nothing anywhere + * before run time, so a deploy went live with a block that failed on every run. + */ + it('reports a string reference written unquoted in a JSON field, whoever the caller is', async () => { + const start = { + ...block('start', 'start_trigger'), + subBlocks: { inputFormat: { value: [{ name: 'order_id', type: 'string' }] } }, + } + const insert = { + ...block('insert', 'table_v2'), + subBlocks: { data: { value: '{"order_id": }' } }, + } + const report = await buildWorkflowLintReport( + { blocks: { start, insert }, edges: [edge('start', 'insert')] } as never, + { ...scope, subjectUserId: null } + ) + + expect(report.unresolvedReferences).toEqual([ + expect.objectContaining({ + blockId: 'insert', + field: 'data', + kind: 'block-output', + value: [''], + }), + ]) + }) +}) diff --git a/apps/sim/lib/workflows/editing/lint-report.ts b/apps/sim/lib/workflows/editing/lint-report.ts index b2a850c83c2..5963ae669b4 100644 --- a/apps/sim/lib/workflows/editing/lint-report.ts +++ b/apps/sim/lib/workflows/editing/lint-report.ts @@ -5,6 +5,7 @@ import { getTableById } from '@/lib/table/service' import { collectDanglingBlockOutputReferences, collectTableBlockFieldIssues, + collectUnquotedJsonStringReferences, collectWorkflowFieldIssues, collectWorkflowTableIds, hasWorkflowEntryBlock, @@ -121,10 +122,14 @@ export async function buildWorkflowLintReport( ...(options.tables?.unresolvedReferences ?? []), ] - // Pure graph check, so it runs for every caller: a dangling block-output + // Pure graph checks, so they run for every caller: a dangling block-output // reference passes literal text through at run time on the surfaces that - // do not fail loudly (API bodies, agent prompts). - unresolvedReferences.push(...collectDanglingBlockOutputReferences(graph)) + // do not fail loudly (API bodies, agent prompts), and an unquoted string + // reference makes a JSON field unparseable only once the block runs. + unresolvedReferences.push( + ...collectDanglingBlockOutputReferences(graph), + ...collectUnquotedJsonStringReferences(graph) + ) if (scope.subjectUserId) { for (const collect of [collectUnresolvedReferences, collectUnresolvedAgentToolReferences]) { diff --git a/apps/sim/lib/workflows/editing/lint.ts b/apps/sim/lib/workflows/editing/lint.ts index c0da8fa036f..1525cb7bd09 100644 --- a/apps/sim/lib/workflows/editing/lint.ts +++ b/apps/sim/lib/workflows/editing/lint.ts @@ -5,6 +5,7 @@ import { } from '@/lib/table/query-builder/field-names' import { getEffectiveBlockOutputs, + getEffectiveBlockOutputType, getResponseFormatOutputs, } from '@/lib/workflows/blocks/block-outputs' import { getBlock } from '@/blocks' @@ -517,7 +518,7 @@ export function formatWorkflowLintMessage(lint: WorkflowLintIssueView) { const blockOutputRefs = unresolved.filter((ref) => ref.kind === 'block-output') if (blockOutputRefs.length > 0) { parts.push( - `Block output references that will not resolve: ${blockOutputRefs + `Block output references that will not work as written: ${blockOutputRefs .map( (ref) => `"${ref.blockName || ref.blockId}".${ref.field} ${ @@ -606,6 +607,15 @@ function firstOutputSegment(token: string): string | undefined { * whose `responseFormat` is set but cannot be parsed, since its fields are * decided when that schema resolves; and any type that declares no outputs. */ +/** A block's sub-blocks without empty slots, as the output-schema helpers read them. */ +function subBlockValues(block: BlockState): Record { + const subBlocks: Record = {} + for (const [id, subBlock] of Object.entries(block.subBlocks ?? {})) { + if (subBlock) subBlocks[id] = subBlock + } + return subBlocks +} + function declaredOutputKeys(block: BlockState): string[] | undefined { const type = block.type if (!type || block.triggerMode === true || isTriggerBlockType(type)) return undefined @@ -613,10 +623,7 @@ function declaredOutputKeys(block: BlockState): string[] | undefined { const config = getBlock(type) if (!config || config.category === 'triggers') return undefined - const subBlocks: Record = {} - for (const [id, subBlock] of Object.entries(block.subBlocks ?? {})) { - if (subBlock) subBlocks[id] = subBlock - } + const subBlocks = subBlockValues(block) if (type === 'agent') { const responseFormat = subBlocks.responseFormat?.value @@ -651,15 +658,34 @@ function quoteList(values: Iterable): string { return [...values].map((value) => `"${value}"`).join(', ') } -export function collectDanglingBlockOutputReferences( - workflowState: Pick -): WorkflowLintUnresolvedReference[] { - const blocks = (workflowState.blocks || {}) as Record +/** Block ids keyed by id and by normalized name, the two heads a reference may use. */ +function referenceTargetIndex(blocks: Record): Map { const targetByKey = new Map() for (const [id, block] of Object.entries(blocks)) { targetByKey.set(id, id) if (block.name) targetByKey.set(normalizeName(block.name), id) } + return targetByKey +} + +/** + * The block a `block.path` token names: a block id, `null` for a head that + * names no block, or `undefined` for a special prefix (`loop`, `variable`, …). + */ +function referenceTarget( + token: string, + targetByKey: Map +): string | null | undefined { + const head = token.split('.')[0] ?? '' + if ((SPECIAL_REFERENCE_PREFIXES as readonly string[]).includes(head)) return undefined + return targetByKey.get(head) ?? targetByKey.get(normalizeName(head)) ?? null +} + +export function collectDanglingBlockOutputReferences( + workflowState: Pick +): WorkflowLintUnresolvedReference[] { + const blocks = (workflowState.blocks || {}) as Record + const targetByKey = referenceTargetIndex(blocks) /** Output keys per referenced block; `null` once found not knowable. */ const outputKeysByTarget = new Map() const outputKeysFor = (targetId: string): string[] | null => { @@ -680,10 +706,9 @@ export function collectDanglingBlockOutputReferences( for (const leaf of leaves) { for (const token of referenceCandidates(leaf, subBlockId === 'code')) { if (!token || !REF_TOKEN_SHAPE.test(token)) continue - const head = token.split('.')[0] ?? '' - if ((SPECIAL_REFERENCE_PREFIXES as readonly string[]).includes(head)) continue - const targetId = targetByKey.get(head) ?? targetByKey.get(normalizeName(head)) - if (targetId === undefined) { + const targetId = referenceTarget(token, targetByKey) + if (targetId === undefined) continue + if (targetId === null) { dangling.add(`<${token}>`) continue } @@ -728,3 +753,77 @@ export function collectDanglingBlockOutputReferences( } return findings } + +/** + * `block.path` bodies of the reference tokens in JSON text that sit outside + * every string literal, tokenized with the runtime's own reference scanner. + * Tokens inside a literal, escaped quotes included, are already strings in the + * parsed document. + */ +function unquotedJsonReferenceTokens(json: string): string[] { + const unquoted: string[] = [] + let cursor = 0 + let inString = false + for (const token of findWorkflowReferenceTokens(json)) { + for (; cursor < token.start; cursor++) { + const char = json[cursor] + if (inString && char === '\\') cursor++ + else if (char === '"') inString = !inString + } + cursor = token.end + if (inString || token.kind !== 'workflow') continue + const body = token.value.slice(REFERENCE.START.length, -REFERENCE.END.length) + if (REF_TOKEN_SHAPE.test(body)) unquoted.push(body) + } + return unquoted +} + +/** + * References to string outputs written unquoted in a JSON field. + * + * Outside Function code a reference is replaced by its raw text, so + * `{"id": }` becomes `{"id": ord-1}` and the block fails to + * parse it at run time, while lint, deploy, and every earlier run of a draft + * that never reached the block stay clean. Only references whose declared + * output type is `string` are reported: numbers, booleans, and objects already + * resolve to JSON values, and an undeclared type cannot be judged here. + */ +export function collectUnquotedJsonStringReferences( + workflowState: Pick +): WorkflowLintUnresolvedReference[] { + const blocks = (workflowState.blocks || {}) as Record + const targetByKey = referenceTargetIndex(blocks) + const findings: WorkflowLintUnresolvedReference[] = [] + for (const [blockId, block] of Object.entries(blocks)) { + const jsonFields = (block.type ? getBlock(block.type)?.subBlocks : undefined)?.filter( + (subBlock) => subBlock.type === 'code' && subBlock.language === 'json' + ) + for (const { id: field } of jsonFields ?? []) { + const json = block.subBlocks?.[field]?.value + if (typeof json !== 'string') continue + const unquoted = new Set() + for (const token of unquotedJsonReferenceTokens(json)) { + const targetId = referenceTarget(token, targetByKey) + const target = targetId ? blocks[targetId] : undefined + if (!target?.type) continue + const path = token.slice(token.indexOf('.') + 1) + const type = getEffectiveBlockOutputType(target.type, path, subBlockValues(target), { + triggerMode: target.triggerMode === true, + preferToolOutputs: true, + includeHidden: true, + }) + if (type === 'string') unquoted.add(`<${token}>`) + } + if (unquoted.size === 0) continue + const value = [...unquoted] + findings.push({ + ...blockRef(blockId, block), + field, + value, + kind: 'block-output', + reason: `unquoted-json-string: these references resolve to text, which is inserted without quotes, so the field is not valid JSON at run time unless the text is itself JSON. Quote each one, e.g. "${value[0]}".`, + }) + } + } + return findings +} From 5f67bf1c742e0bf35bacb0d8ae3fe7664e2646e6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 8 Oct 2026 08:53:47 -0700 Subject: [PATCH 2/3] fix(workflows): lint only the JSON fields the operation sends, and lint deploys as the acting user --- .../lib/workflows/application/deployments.ts | 10 +++++--- .../application/workflow-deployments.test.ts | 11 ++++---- .../editing/json-literal-refs.test.ts | 25 ++++++++++++++++--- apps/sim/lib/workflows/editing/lint.ts | 12 +++++++-- 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/apps/sim/lib/workflows/application/deployments.ts b/apps/sim/lib/workflows/application/deployments.ts index 903214d2dae..d9af5605018 100644 --- a/apps/sim/lib/workflows/application/deployments.ts +++ b/apps/sim/lib/workflows/application/deployments.ts @@ -1,5 +1,9 @@ import { AuditAction, AuditResourceType } from '@sim/audit' -import { resolvePrincipalAttribution, toPrincipalActor } from '@sim/auth/principal' +import { + resolvePrincipalAttribution, + resolvePrincipalSubjectUserId, + toPrincipalActor, +} from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/workflow' import { getErrorMessage } from '@sim/utils/errors' @@ -108,7 +112,7 @@ async function requireMutableWorkflow(workflowId: string): Promise { async function deployedVersionLintWarning( context: ActiveWorkflowApplicationContext, deploymentVersionId: string | undefined, - subjectUserId: string + subjectUserId: string | null ): Promise { if (!deploymentVersionId) return undefined try { @@ -156,7 +160,7 @@ export const deployWorkflow = defineAuthorizedWorkflowUseCase({ const lintWarning = await deployedVersionLintWarning( context, result.deploymentVersionId, - attribution.attributedUserId + resolvePrincipalSubjectUserId(principal) ?? null ) return { ...result, diff --git a/apps/sim/lib/workflows/application/workflow-deployments.test.ts b/apps/sim/lib/workflows/application/workflow-deployments.test.ts index be47485cc11..4bae2602a00 100644 --- a/apps/sim/lib/workflows/application/workflow-deployments.test.ts +++ b/apps/sim/lib/workflows/application/workflow-deployments.test.ts @@ -341,7 +341,7 @@ describe('workflow deployment application use cases', () => { ], } - it('reports them as a deploy warning, linted as the deploying user', async () => { + it('reports them as a deploy warning, linted as the acting user', async () => { mockDeploy.mockResolvedValueOnce({ success: true, version: 4, @@ -350,17 +350,16 @@ describe('workflow deployment application use cases', () => { 'Deployment activation completed, and post-activation notifications are queued.', ], }) - mocks.buildWorkflowLintReport.mockResolvedValueOnce(unquotedRowJson) + mocks.buildWorkflowLintReport.mockImplementation( + async (_graph: unknown, scope: { subjectUserId: string | null }) => + scope.subjectUserId === 'session-user' ? unquotedRowJson : cleanLint + ) const result = await deployWorkflow.execute({ principal: createSessionPrincipal({ userId: 'session-user' }), input: { workflowId: 'workflow-1', requestId: 'request-10' }, }) - expect(mocks.buildWorkflowLintReport).toHaveBeenCalledWith( - { blocks: {}, edges: [] }, - expect.objectContaining({ workflowId: 'workflow-1', subjectUserId: 'session-user' }) - ) expect(result.warnings).toHaveLength(2) expect(result.warnings?.[1]).toContain('"Insert Order".data ') }) diff --git a/apps/sim/lib/workflows/editing/json-literal-refs.test.ts b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts index 3879d5bcf27..f6f8056491c 100644 --- a/apps/sim/lib/workflows/editing/json-literal-refs.test.ts +++ b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts @@ -31,7 +31,12 @@ const MOCK_BLOCKS = vi.hoisted( category: 'blocks', subBlocks: [ { id: 'operation', type: 'dropdown' }, - { id: 'data', type: 'code', language: 'json' }, + { + id: 'data', + type: 'code', + language: 'json', + condition: { field: 'operation', value: ['insert_row', 'update_row'] }, + }, { id: 'rows', type: 'code', language: 'json' }, ], outputs: {}, @@ -87,8 +92,12 @@ function graph( return { blocks } as Parameters[0] } -function insertRow(data: unknown) { - return { type: 'table_v2', name: 'Insert Order', subBlocks: { data: { value: data } } } +function insertRow(data: unknown, operation = 'insert_row') { + return { + type: 'table_v2', + name: 'Insert Order', + subBlocks: { operation: { value: operation }, data: { value: data } }, + } } describe('collectUnquotedJsonStringReferences', () => { @@ -196,4 +205,14 @@ describe('collectUnquotedJsonStringReferences', () => { ) expect(findings).toHaveLength(0) }) + + it('ignores a JSON field the selected operation does not send', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + insert: insertRow('{"order_id": }', 'get_schema'), + }) + ) + expect(findings).toHaveLength(0) + }) }) diff --git a/apps/sim/lib/workflows/editing/lint.ts b/apps/sim/lib/workflows/editing/lint.ts index 1525cb7bd09..232da0462a3 100644 --- a/apps/sim/lib/workflows/editing/lint.ts +++ b/apps/sim/lib/workflows/editing/lint.ts @@ -798,8 +798,16 @@ export function collectUnquotedJsonStringReferences( const jsonFields = (block.type ? getBlock(block.type)?.subBlocks : undefined)?.filter( (subBlock) => subBlock.type === 'code' && subBlock.language === 'json' ) - for (const { id: field } of jsonFields ?? []) { - const json = block.subBlocks?.[field]?.value + if (!jsonFields?.length) continue + /** Only what the serializer sends: a field the selected operation drops never runs. */ + let params: Record + try { + params = extractBlockParams(block as Parameters[0]) + } catch { + continue + } + for (const field of new Set(jsonFields.map((subBlock) => subBlock.id))) { + const json = params[field] if (typeof json !== 'string') continue const unquoted = new Set() for (const token of unquotedJsonReferenceTokens(json)) { From fd2ab618f4a2bc1dc0bc734889baa6d2c8b91380 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 8 Oct 2026 09:01:12 -0700 Subject: [PATCH 3/3] fix(workflows): check JSON editors sent under a canonical param --- .../lib/workflows/application/deployments.ts | 6 ++-- .../editing/json-literal-refs.test.ts | 36 ++++++++++++++++++- apps/sim/lib/workflows/editing/lint.ts | 12 +++++-- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/apps/sim/lib/workflows/application/deployments.ts b/apps/sim/lib/workflows/application/deployments.ts index d9af5605018..fb13b7e8de4 100644 --- a/apps/sim/lib/workflows/application/deployments.ts +++ b/apps/sim/lib/workflows/application/deployments.ts @@ -102,7 +102,9 @@ async function requireMutableWorkflow(workflowId: string): Promise { } /** - * The lint findings of the version a deploy just made live, as one warning. + * The lint findings of the version a deploy admitted, as one warning. That + * version serves callers once activation completes, so it is linted even while + * activation is still pending. * * Deploy does not refuse on lint: findings are advisory, and some depend on the * identity that runs the workflow. But a caller that deployed without linting @@ -127,7 +129,7 @@ async function deployedVersionLintWarning( ) ) if (!hasWorkflowLintIssues(report)) return undefined - return `The deployed version has lint findings and may fail when it runs. ${formatWorkflowLintMessage(report)}` + return `The version this deploy publishes has lint findings and may fail when it runs. ${formatWorkflowLintMessage(report)}` } catch (error) { logger.warn('Deployed version lint failed', { workflowId: context.workflowId, diff --git a/apps/sim/lib/workflows/editing/json-literal-refs.test.ts b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts index f6f8056491c..4627a111bc0 100644 --- a/apps/sim/lib/workflows/editing/json-literal-refs.test.ts +++ b/apps/sim/lib/workflows/editing/json-literal-refs.test.ts @@ -38,6 +38,19 @@ const MOCK_BLOCKS = vi.hoisted( condition: { field: 'operation', value: ['insert_row', 'update_row'] }, }, { id: 'rows', type: 'code', language: 'json' }, + { + id: 'filterBuilder', + type: 'filter-builder', + canonicalParamId: 'filterInput', + mode: 'basic', + }, + { + id: 'filter', + type: 'code', + language: 'json', + canonicalParamId: 'filterInput', + mode: 'advanced', + }, ], outputs: {}, }, @@ -87,7 +100,10 @@ const START = { } function graph( - blocks: Record }> + blocks: Record< + string, + { type?: string; name?: string; subBlocks?: Record; data?: unknown } + > ) { return { blocks } as Parameters[0] } @@ -215,4 +231,22 @@ describe('collectUnquotedJsonStringReferences', () => { ) expect(findings).toHaveLength(0) }) + + it('checks a JSON editor that is sent under its canonical parameter', () => { + const findings = collectUnquotedJsonStringReferences( + graph({ + start: START, + query: { + type: 'table_v2', + name: 'Find Order', + data: { canonicalModes: { filterInput: 'advanced' } }, + subBlocks: { + operation: { value: 'query_rows' }, + filter: { value: '{"field": "order_id", "op": "eq", "value": }' }, + }, + }, + }) + ) + expect(findings).toMatchObject([{ blockId: 'query', field: 'filter' }]) + }) }) diff --git a/apps/sim/lib/workflows/editing/lint.ts b/apps/sim/lib/workflows/editing/lint.ts index 232da0462a3..b714bc002f2 100644 --- a/apps/sim/lib/workflows/editing/lint.ts +++ b/apps/sim/lib/workflows/editing/lint.ts @@ -799,15 +799,21 @@ export function collectUnquotedJsonStringReferences( (subBlock) => subBlock.type === 'code' && subBlock.language === 'json' ) if (!jsonFields?.length) continue - /** Only what the serializer sends: a field the selected operation drops never runs. */ + /** + * Only what the serializer sends: a field the selected operation or mode + * drops never runs, and a canonical member is sent under its canonical id. + */ let params: Record try { params = extractBlockParams(block as Parameters[0]) } catch { continue } - for (const field of new Set(jsonFields.map((subBlock) => subBlock.id))) { - const json = params[field] + const sentParamByField = new Map( + jsonFields.map((subBlock) => [subBlock.id, subBlock.canonicalParamId ?? subBlock.id]) + ) + for (const [field, param] of sentParamByField) { + const json = params[param] if (typeof json !== 'string') continue const unquoted = new Set() for (const token of unquotedJsonReferenceTokens(json)) {