Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/docs/openapi-v2-workflows.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/lib/api/contracts/v2/workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
59 changes: 58 additions & 1 deletion apps/sim/lib/workflows/application/deployments.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
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'
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,
Expand All @@ -19,9 +29,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
Expand Down Expand Up @@ -88,6 +101,44 @@ async function requireMutableWorkflow(workflowId: string): Promise<void> {
}
}

/**
* 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
* 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 | null
): Promise<string | undefined> {
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 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,
error: getErrorMessage(error),
})
return undefined
}
}

export const deployWorkflow = defineAuthorizedWorkflowUseCase({
operation: workflowOperations.deploy,
resolveContext: resolvePrincipalWorkflowContext<DeployWorkflowInput>,
Expand All @@ -108,8 +159,14 @@ export const deployWorkflow = defineAuthorizedWorkflowUseCase({
idempotencyKey: input.idempotencyKey,
})
if (!result.success) throwDeploymentFailure(result, 'Failed to deploy workflow')
const lintWarning = await deployedVersionLintWarning(
context,
result.deploymentVersionId,
Comment thread
waleedlatif1 marked this conversation as resolved.
resolvePrincipalSubjectUserId(principal) ?? null
)
return {
...result,
warnings: lintWarning ? [...(result.warnings ?? []), lintWarning] : result.warnings,
workflowId: context.workflowId,
workspaceId: context.workspaceId,
}
Expand Down
89 changes: 89 additions & 0 deletions apps/sim/lib/workflows/application/workflow-deployments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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' }),
Expand Down Expand Up @@ -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: [],
Expand All @@ -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)(
Expand Down Expand Up @@ -297,4 +321,69 @@ 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: ['<start.order_id>'],
kind: 'block-output' as const,
reason: 'unquoted-json-string: quote it',
},
],
}

it('reports them as a deploy warning, linted as the acting user', async () => {
mockDeploy.mockResolvedValueOnce({
success: true,
version: 4,
deploymentVersionId: 'version-4',
warnings: [
'Deployment activation completed, and post-activation notifications are queued.',
],
})
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(result.warnings).toHaveLength(2)
expect(result.warnings?.[1]).toContain('"Insert Order".data <start.order_id>')
})

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: [] })
})
})
})
Loading
Loading