From 3ecef5c70c8a51ebdfdcd10f6c4b1d4e9bae2c40 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 3 Nov 2026 15:32:48 -0800 Subject: [PATCH] fix(workflows): keep the Run-button lint finding advisory and point at lint from apply warnings --- apps/docs/openapi-v2-workflows.json | 8 +- .../lib/api/contracts/v2/openapi/workflows.ts | 2 +- apps/sim/lib/api/contracts/v2/workflows.ts | 4 +- .../apply-workflow-operations.test.ts | 9 +- .../application/apply-workflow-operations.ts | 8 +- apps/sim/lib/workflows/editing/lint.test.ts | 53 +++++ apps/sim/lib/workflows/editing/lint.ts | 223 ++++++++++-------- 7 files changed, 191 insertions(+), 116 deletions(-) diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index 44fed572f91..7768b388293 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -739,7 +739,7 @@ "notes": [] }, "warnings": [ - "Workflow lint found issues. Fix these before continuing: Blocks missing required fields: \"Triage\" (systemPrompt)" + "Workflow lint found issues (Blocks missing required fields: 1). The findings are in `lint`." ], "needsRedeployment": true, "dryRun": false @@ -8885,7 +8885,7 @@ "type": "null" } ], - "description": "Why a manual run from the editor's Run button would be refused before it starts: no enabled trigger it can start from, more than one API trigger, or a selected trigger with no outgoing connection. `null` when no manual-run issue is reported: the run can begin, only a chat trigger can start the workflow, or the graph is empty (reported in `notes` instead)." + "description": "Why a manual run from the editor's Run button would be refused before it starts: no enabled trigger it can start from, more than one API trigger, or a selected trigger with no outgoing connection. The API run endpoints do not apply these checks. `null` when no manual-run issue is reported: the run can begin, only a chat trigger can start the workflow, or the graph is empty (reported in `notes` instead)." }, "notes": { "type": "array", @@ -9537,7 +9537,7 @@ "items": { "type": "string" }, - "description": "Non-fatal notes about blocks and edges that were normalized or dropped before persistence and, when `lint` has findings, one summary of them, so the findings are not lost to a caller that reads only `warnings`. Empty when there is nothing to report." + "description": "Non-fatal notes about blocks and edges that were normalized or dropped before persistence and, when `lint` has findings, one line counting them by category, so a caller that reads only `warnings` still learns they exist. The findings themselves are in `lint`. Empty when there is nothing to report." }, "needsRedeployment": { "type": "boolean", @@ -9673,7 +9673,7 @@ "notes": [] }, "warnings": [ - "Workflow lint found issues. Fix these before continuing: Blocks missing required fields: \"Triage\" (systemPrompt)" + "Workflow lint found issues (Blocks missing required fields: 1). The findings are in `lint`." ], "needsRedeployment": true, "dryRun": false diff --git a/apps/sim/lib/api/contracts/v2/openapi/workflows.ts b/apps/sim/lib/api/contracts/v2/openapi/workflows.ts index d71fd982169..5637d6fe7fd 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/workflows.ts @@ -436,7 +436,7 @@ const declaredRoutes = [ notes: [], }, warnings: [ - 'Workflow lint found issues. Fix these before continuing: Blocks missing required fields: "Triage" (systemPrompt)', + 'Workflow lint found issues (Blocks missing required fields: 1). The findings are in `lint`.', ], needsRedeployment: true, dryRun: false, diff --git a/apps/sim/lib/api/contracts/v2/workflows.ts b/apps/sim/lib/api/contracts/v2/workflows.ts index b79f8a63fde..ecf991224de 100644 --- a/apps/sim/lib/api/contracts/v2/workflows.ts +++ b/apps/sim/lib/api/contracts/v2/workflows.ts @@ -454,7 +454,7 @@ const v2WorkflowLintSchema = z }) .nullable() .describe( - "Why a manual run from the editor's Run button would be refused before it starts: no enabled trigger it can start from, more than one API trigger, or a selected trigger with no outgoing connection. `null` when no manual-run issue is reported: the run can begin, only a chat trigger can start the workflow, or the graph is empty (reported in `notes` instead)." + "Why a manual run from the editor's Run button would be refused before it starts: no enabled trigger it can start from, more than one API trigger, or a selected trigger with no outgoing connection. The API run endpoints do not apply these checks. `null` when no manual-run issue is reported: the run can begin, only a chat trigger can start the workflow, or the graph is empty (reported in `notes` instead)." ), notes: z.array(z.string()).describe('Advisory notes about the report itself.'), }) @@ -3646,7 +3646,7 @@ export const v2ApplyWorkflowOperationsDataSchema = v2WorkflowGraphWriteResultSch warnings: z .array(z.string()) .describe( - 'Non-fatal notes about blocks and edges that were normalized or dropped before persistence and, when `lint` has findings, one summary of them, so the findings are not lost to a caller that reads only `warnings`. Empty when there is nothing to report.' + 'Non-fatal notes about blocks and edges that were normalized or dropped before persistence and, when `lint` has findings, one line counting them by category, so a caller that reads only `warnings` still learns they exist. The findings themselves are in `lint`. Empty when there is nothing to report.' ), dryRun: z .boolean() 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 0e99cac3182..533f212bcbd 100644 --- a/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts +++ b/apps/sim/lib/workflows/application/apply-workflow-operations.test.ts @@ -269,9 +269,12 @@ describe('applyWorkflowOperations', () => { * An agent piping the apply response through a projection of `warnings` * never saw lint findings, which lived only under `lint`. */ - it('summarizes lint findings in warnings, on commit and on dry run', async () => { + it('points at lint findings from warnings, on commit and on dry run', async () => { const orphan = { blockId: 'block-1', blockName: 'Start', blockType: 'starter' } - mocks.lintGraph.mockReturnValue({ ...EMPTY_GRAPH_LINT, orphanBlocks: [orphan] }) + mocks.lintGraph.mockReturnValue({ + ...EMPTY_GRAPH_LINT, + orphanBlocks: [orphan, { ...orphan, blockId: 'block-2', blockName: 'Fn' }], + }) for (const dryRun of [false, true]) { const result = await applyWorkflowOperations.execute({ @@ -279,7 +282,7 @@ describe('applyWorkflowOperations', () => { input: { workflowId: 'workflow-1', operations, dryRun }, }) expect(result.warnings).toEqual([ - 'Workflow lint found issues. Fix these before continuing: Blocks with no incoming edge: "Start" (starter)', + 'Workflow lint found issues (Blocks with no incoming edge: 2). The findings are in `lint`.', ]) } }) diff --git a/apps/sim/lib/workflows/application/apply-workflow-operations.ts b/apps/sim/lib/workflows/application/apply-workflow-operations.ts index fe85bc5a1dd..92656e7d2f6 100644 --- a/apps/sim/lib/workflows/application/apply-workflow-operations.ts +++ b/apps/sim/lib/workflows/application/apply-workflow-operations.ts @@ -39,7 +39,7 @@ import { } from '@/lib/workflows/editing/block-enablement' import { applyOperationsToWorkflowState } from '@/lib/workflows/editing/engine' import type { WorkflowLintReport } from '@/lib/workflows/editing/lint' -import { formatWorkflowLintMessage, hasWorkflowLintIssues } from '@/lib/workflows/editing/lint' +import { hasWorkflowLintIssues, summarizeWorkflowLintIssues } from '@/lib/workflows/editing/lint' import { buildWorkflowLintReport } from '@/lib/workflows/editing/lint-report' import { operationsReferenceSimSandbox } from '@/lib/workflows/editing/sandbox-projection' import { @@ -365,10 +365,10 @@ export const applyWorkflowOperations = defineAuthorizedWorkflowUseCase({ input.checkAborted?.() /** - * The findings also ride in `warnings`, where a caller that projects the - * response down to its warnings still sees them. + * A caller that projects the response down to its warnings still learns + * the findings exist; the findings themselves stay only in `lint`. */ - const lintWarnings = hasWorkflowLintIssues(lint) ? [formatWorkflowLintMessage(lint)] : [] + const lintWarnings = hasWorkflowLintIssues(lint) ? [summarizeWorkflowLintIssues(lint)] : [] /** * A dry run still runs the whole engine — operations are applied, refusals diff --git a/apps/sim/lib/workflows/editing/lint.test.ts b/apps/sim/lib/workflows/editing/lint.test.ts index 1f4a3ad95d2..084c59e068e 100644 --- a/apps/sim/lib/workflows/editing/lint.test.ts +++ b/apps/sim/lib/workflows/editing/lint.test.ts @@ -6,8 +6,10 @@ import { collectTableBlockFieldIssues, collectWorkflowFieldIssues, collectWorkflowTableIds, + formatWorkflowLintMessage, hasWorkflowLintIssues, lintEditedWorkflowState, + summarizeWorkflowLintIssues, } from './lint' const mockGetToolParams = getToolParams as Mock @@ -535,3 +537,54 @@ describe('collectWorkflowTableIds', () => { expect(ids).toEqual(['tbl_1']) }) }) + +/** + * The agent's own runs name their trigger and skip the Run button's checks, so + * a Run-button refusal told under "Fix these before continuing" pushed agents + * to wire a stray Start into the graph instead of removing it. + */ +describe('formatWorkflowLintMessage', () => { + const clean = { + sources: [], + sinks: [], + orphanBlocks: [], + emptyOutgoingPorts: [], + invalidBranchPorts: [], + invalidConnectionTargets: [], + } + const strayStart = { + reason: 'trigger-not-connected' as const, + message: 'Start must be connected to other blocks to execute', + blockId: 'start-1', + blockName: 'Start', + blockType: 'start_trigger', + } + + it('reports a Run-button refusal as advice outside the blocking findings, naming both remedies', () => { + const message = formatWorkflowLintMessage({ ...clean, manualRunIssue: strayStart }) + expect(message).not.toContain('Fix these before continuing') + expect(message).toBe( + 'The editor\'s Run button would fail: Start must be connected to other blocks to execute. Run starts from "Start" (start_trigger) ahead of any other trigger: connect it, or delete it if the workflow starts from another trigger. Runs that name their trigger are not affected.' + ) + }) + + it('points at a Run-button refusal alone without calling it a lint issue', () => { + expect(summarizeWorkflowLintIssues({ ...clean, manualRunIssue: strayStart })).toBe( + "The editor's Run button would fail. The findings are in `lint`." + ) + }) + + it('keeps blocking findings first and the Run-button advice after them', () => { + const message = formatWorkflowLintMessage({ + ...clean, + orphanBlocks: [{ blockId: 'fn', blockName: 'Fn', blockType: 'function' }], + manualRunIssue: { + reason: 'multiple-api-triggers', + message: 'Multiple API Trigger blocks found. Keep only one.', + }, + }) + expect(message).toBe( + 'Workflow lint found issues. Fix these before continuing: Blocks with no incoming edge: "Fn" (function). The editor\'s Run button would fail: Multiple API Trigger blocks found. Keep only one.' + ) + }) +}) diff --git a/apps/sim/lib/workflows/editing/lint.ts b/apps/sim/lib/workflows/editing/lint.ts index 67bf2d37522..0fec0dc00f4 100644 --- a/apps/sim/lib/workflows/editing/lint.ts +++ b/apps/sim/lib/workflows/editing/lint.ts @@ -443,125 +443,144 @@ export function hasWorkflowLintIssues(lint: WorkflowLintIssueView) { ) } -export function formatWorkflowLintMessage(lint: WorkflowLintIssueView) { - const parts: string[] = [] - - if (lint.orphanBlocks.length > 0) { - parts.push( - `Blocks with no incoming edge: ${lint.orphanBlocks - .map((block) => `"${block.blockName || block.blockId}" (${block.blockType || 'unknown'})`) - .join(', ')}` - ) - } - - if (lint.emptyOutgoingPorts.length > 0) { - parts.push( - `Unconnected required subflow start ports: ${lint.emptyOutgoingPorts - .map((port) => `"${port.blockName || port.blockId}".${port.label}`) - .join(', ')}` - ) - } - - if (lint.invalidBranchPorts.length > 0) { - parts.push( - `Invalid condition/router branch handles: ${lint.invalidBranchPorts - .map((port) => `"${port.blockName || port.blockId}" uses "${port.sourceHandle}"`) - .join(', ')}` - ) +/** One category of findings an agent must fix: its label, how many, and a renderer for them. */ +interface WorkflowLintSection { + label: string + count: number + render: () => string +} + +function collectBlockingSections(lint: WorkflowLintIssueView): WorkflowLintSection[] { + const sections: WorkflowLintSection[] = [] + const add = (label: string, items: T[], render: (item: T) => string, separator = ', ') => { + if (items.length > 0) { + sections.push({ + label, + count: items.length, + render: () => items.map(render).join(separator), + }) + } } - if (lint.invalidConnectionTargets.length > 0) { - parts.push( - `Connections pointing at missing blocks: ${lint.invalidConnectionTargets - .map((edge) => `${edge.sourceBlockId} -> ${edge.targetBlockId}`) - .join(', ')}` - ) - } + add( + 'Blocks with no incoming edge', + lint.orphanBlocks, + (block) => `"${block.blockName || block.blockId}" (${block.blockType || 'unknown'})` + ) + add( + 'Unconnected required subflow start ports', + lint.emptyOutgoingPorts, + (port) => `"${port.blockName || port.blockId}".${port.label}` + ) + add( + 'Invalid condition/router branch handles', + lint.invalidBranchPorts, + (port) => `"${port.blockName || port.blockId}" uses "${port.sourceHandle}"` + ) + add( + 'Connections pointing at missing blocks', + lint.invalidConnectionTargets, + (edge) => `${edge.sourceBlockId} -> ${edge.targetBlockId}` + ) const fieldIssues = lint.fieldIssues ?? [] - const missing = fieldIssues.filter((issue) => issue.missingRequiredFields.length > 0) - if (missing.length > 0) { - parts.push( - `Blocks missing required fields: ${missing - .map( - (issue) => - `"${issue.blockName || issue.blockId}" (${issue.missingRequiredFields.join(', ')})` - ) - .join(', ')}` - ) - } - - const inactive = fieldIssues.filter((issue) => issue.inactiveModeValues.length > 0) - if (inactive.length > 0) { - parts.push( - `Values set on the inactive field mode (they will not resolve): ${inactive + add( + 'Blocks missing required fields', + fieldIssues.filter((issue) => issue.missingRequiredFields.length > 0), + (issue) => `"${issue.blockName || issue.blockId}" (${issue.missingRequiredFields.join(', ')})` + ) + add( + 'Values set on the inactive field mode (they will not resolve)', + fieldIssues.filter((issue) => issue.inactiveModeValues.length > 0), + (issue) => + `"${issue.blockName || issue.blockId}" (${issue.inactiveModeValues .map( - (issue) => - `"${issue.blockName || issue.blockId}" (${issue.inactiveModeValues - .map( - (v) => - `${v.inactiveMemberId}: move the value to "${v.activeMemberId ?? v.canonicalId}"` - ) - .join('; ')})` + (v) => `${v.inactiveMemberId}: move the value to "${v.activeMemberId ?? v.canonicalId}"` ) - .join(', ')}` - ) - } + .join('; ')})` + ) const unresolved = lint.unresolvedReferences ?? [] - const credResourceRefs = unresolved.filter( - (ref) => ref.kind === 'credential' || ref.kind === 'resource' + const renderRef = (ref: WorkflowLintUnresolvedReference) => + `"${ref.blockName || ref.blockId}".${ref.field} (${ref.reason})` + add( + 'Credential/resource references that do not resolve', + unresolved.filter((ref) => ref.kind === 'credential' || ref.kind === 'resource'), + renderRef ) - if (credResourceRefs.length > 0) { - parts.push( - `Credential/resource references that do not resolve: ${credResourceRefs - .map((ref) => `"${ref.blockName || ref.blockId}".${ref.field} (${ref.reason})`) - .join(', ')}` - ) - } - - const toolSkillRefs = unresolved.filter( - (ref) => ref.kind === 'custom-tool' || ref.kind === 'mcp-tool' || ref.kind === 'skill' + add( + 'Agent tool/skill references that do not resolve (they will not attach at runtime)', + unresolved.filter( + (ref) => ref.kind === 'custom-tool' || ref.kind === 'mcp-tool' || ref.kind === 'skill' + ), + renderRef + ) + add( + 'Block output references that will not work as written', + unresolved.filter((ref) => ref.kind === 'block-output'), + (ref) => + `"${ref.blockName || ref.blockId}".${ref.field} ${ + Array.isArray(ref.value) ? ref.value.join(', ') : ref.value + } (${ref.reason})`, + '; ' + ) + add( + "Table filter/sort fields that are not columns of the referenced table (the run will fail in the block's error edge)", + lint.tableFieldIssues ?? [], + (issue) => `"${issue.blockName || issue.blockId}".${issue.field} (table "${issue.tableName}")` ) - if (toolSkillRefs.length > 0) { - parts.push( - `Agent tool/skill references that do not resolve (they will not attach at runtime): ${toolSkillRefs - .map((ref) => `"${ref.blockName || ref.blockId}".${ref.field} (${ref.reason})`) - .join(', ')}` - ) - } - const blockOutputRefs = unresolved.filter((ref) => ref.kind === 'block-output') - if (blockOutputRefs.length > 0) { - parts.push( - `Block output references that will not work as written: ${blockOutputRefs - .map( - (ref) => - `"${ref.blockName || ref.blockId}".${ref.field} ${ - Array.isArray(ref.value) ? ref.value.join(', ') : ref.value - } (${ref.reason})` - ) + return sections +} + +const RUN_BUTTON_REFUSAL = "The editor's Run button would fail" + +/** + * What the editor's Run button would refuse. Runs the agent or API start name + * their trigger and skip these checks, so this is advice for the person who + * presses Run, never a finding that blocks the caller. + */ +function describeManualRunIssue(issue: ManualRunEntryIssue): string { + const refusal = `${RUN_BUTTON_REFUSAL}: ${issue.message.replace(/\.$/, '')}.` + if (issue.reason !== 'trigger-not-connected' || !issue.blockId) return refusal + return `${refusal} Run starts from "${issue.blockName || issue.blockId}" (${issue.blockType}) ahead of any other trigger: connect it, or delete it if the workflow starts from another trigger. Runs that name their trigger are not affected.` +} + +/** + * The full findings as one message: what must be fixed first, then the + * Run-button advice. Empty when there are no findings. + */ +export function formatWorkflowLintMessage(lint: WorkflowLintIssueView) { + const sections = collectBlockingSections(lint) + const sentences: string[] = [] + if (sections.length > 0) { + sentences.push( + `Workflow lint found issues. Fix these before continuing: ${sections + .map((section) => `${section.label}: ${section.render()}`) .join('; ')}` ) } + if (lint.manualRunIssue) sentences.push(describeManualRunIssue(lint.manualRunIssue)) + return sentences.join('. ') +} - const tableFields = lint.tableFieldIssues ?? [] - if (tableFields.length > 0) { - parts.push( - `Table filter/sort fields that are not columns of the referenced table (the run will fail in the block's error edge): ${tableFields - .map( - (issue) => - `"${issue.blockName || issue.blockId}".${issue.field} (table "${issue.tableName}")` - ) - .join(', ')}` +/** + * A one-line pointer to the findings, counted by category, for a response that + * already carries the full report under `lint`. + */ +export function summarizeWorkflowLintIssues(lint: WorkflowLintIssueView) { + const sentences: string[] = [] + const sections = collectBlockingSections(lint) + if (sections.length > 0) { + sentences.push( + `Workflow lint found issues (${sections + .map((section) => `${section.label}: ${section.count}`) + .join('; ')}).` ) } - - if (lint.manualRunIssue) { - parts.push(`A manual run would fail: ${lint.manualRunIssue.message}`) - } - - return `Workflow lint found issues. Fix these before continuing: ${parts.join('; ')}` + if (lint.manualRunIssue) sentences.push(`${RUN_BUTTON_REFUSAL}.`) + sentences.push('The findings are in `lint`.') + return sentences.join(' ') } /**