GeezerVison is showing github.com — fix(deploy): resolve the error-output flag from edges on both sides of change detection by waleedlatif1 · Pull Request #6754 · simstudioai/sim
fix(deploy): resolve the error-output flag from edges on both sides of change detection
#6754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement . We’ll occasionally send you account related emails.
Already on GitHub?
Sign in
to your account
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.ts
(4)
All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -386,6 +386,39 @@ describe('hasWorkflowChanged', () => {
expect(hasWorkflowChanged(unset, withErrorFlag(false))).toBe(false)
expect(hasWorkflowChanged(unset, withErrorFlag(true))).toBe(true)
})
/**
* `setBlockErrorEnabled` leaves existing error edges in place, so a block can
* hold `errorEnabled: false` with a connected error edge. Only the deployed
* side is backfilled (`materializeDeploymentState`), so reading the flag alone
* makes that block differ from itself, and redeploying cannot clear it — the
* snapshot stores the live `false` that the next read backfills to `true`.
*/
const withErrorEdge = (errorEnabled: boolean) =>
createWorkflowState({
blocks: {
block1: { ...createBlock('block1'), errorEnabled },
block2: createBlock('block2'),
},
edges: [
{ id: 'e1', source: 'block1', sourceHandle: 'error', target: 'block2' },
] as WorkflowState['edges'],
})
it.concurrent('treats a live error edge as the flag being on', () => {
expect(hasWorkflowChanged(withErrorEdge(false), withErrorEdge(true))).toBe(false)
expect(hasWorkflowChanged(withErrorEdge(true), withErrorEdge(false))).toBe(false)
})
it.concurrent('reports no modified block when only the backfilled flag differs', () => {
const summary = generateWorkflowDiffSummary(withErrorEdge(false), withErrorEdge(true))
expect(summary.hasChanges).toBe(false)
expect(summary.modifiedBlocks).toEqual([])
})
it.concurrent('still detects the flag turning on when no error edge exists', () => {
expect(hasWorkflowChanged(withErrorFlag(true), withErrorFlag(false))).toBe(true)
})
})
describe('SubBlock Changes', () => {
Expand Down
Original file line number
Diff line number
Diff line change
@@ -1,5 +1,9 @@
import { createLogger } from '@sim/logger'
import { blockRetryEquals } from '@sim/workflow-types/workflow'
import {
blockRetryEquals,
collectErrorSourceBlockIds,
resolveEffectiveErrorEnabled,
} from '@sim/workflow-types/workflow'
import type { WorkflowState } from '@/stores/workflows/workflow/types'
import {
extractBlockFieldsForComparison,
Expand Down
Expand Up
@@ -132,6 +136,8 @@ export function generateWorkflowDiffSummary(
const previousBlocks = previousState.blocks || {}
const currentBlockIds = new Set(Object.keys(currentBlocks))
const previousBlockIds = new Set(Object.keys(previousBlocks))
const currentErrorSources = collectErrorSourceBlockIds(currentState.edges)
const previousErrorSources = collectErrorSourceBlockIds(previousState.edges)
for (const id of currentBlockIds) {
if (!previousBlockIds.has(id)) {
Expand Down
Expand Up
@@ -173,6 +179,25 @@ export function generateWorkflowDiffSummary(
subBlocks: previousSubBlocks,
} = extractBlockFieldsForComparison(previousBlock)
/**
* Outside the structural gate below: the flag alone can match while the edges
* disagree, and reading it alone pins a block with a stale `errorEnabled: false`
* and a live error edge to "needs redeploy" forever.
*/
const currentErrorEnabled = resolveEffectiveErrorEnabled(currentBlock, id, currentErrorSources)
const previousErrorEnabled = resolveEffectiveErrorEnabled(
previousBlock,
id,
previousErrorSources
)
if (currentErrorEnabled !== previousErrorEnabled) {
changes.push({
field: 'errorEnabled',
oldValue: previousErrorEnabled,
newValue: currentErrorEnabled,
})
}
const normalizedCurrentBlock = { ...currentRest, data: currentDataRest, subBlocks: undefined }
const normalizedPreviousBlock = {
...previousRest,
Expand All
@@ -196,12 +221,8 @@ export function generateWorkflowDiffSummary(
newValue: currentBlock.enabled,
})
}
const blockFields = [
'horizontalHandles',
'advancedMode',
'triggerMode',
'errorEnabled',
] as const
/** `errorEnabled` is compared above, against the edges as well as the flag. */
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const
for (const field of blockFields) {
if (!!currentBlock[field] !== !!previousBlock[field]) {
changes.push({
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -17,7 +17,10 @@ import {
import { saveWorkflowToNormalizedTables as saveWorkflowToNormalizedTablesRaw } from '@sim/workflow-persistence/save'
import type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types'
import type { BlockState, Loop, Parallel, WorkflowState } from '@sim/workflow-types/workflow'
import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow'
import {
collectErrorSourceBlockIds,
normalizeWorkflowEdgeHandles,
} from '@sim/workflow-types/workflow'
import type { InferSelectModel } from 'drizzle-orm'
import { and, desc, eq, inArray, lt, sql } from 'drizzle-orm'
import { LRUCache } from 'lru-cache'
Expand Down
Expand Up
@@ -181,18 +184,18 @@ async function materializeDeploymentState(
*/
const edges = normalizeWorkflowEdgeHandles(state.edges)
/*
/**
* An error edge means the error output is on. Every version before the toggle
* drew that port unconditionally, so a snapshot with such an edge was taken
* from a block that had the output — and the migration backfilling the flag
* only reaches the live tables, never a version's frozen jsonb. Without this
* the deployed side reads `false` against a live `true` and every workflow
* deployed before the toggle asks to be redeployed once. Same rule as
* `workflow-block.tsx` applies at render time; neither may read the flag alone.
* deployed before the toggle asks to be redeployed once. This backfills only
* the deployed side, so change detection must apply `resolveEffectiveErrorEnabled`
* to the live side too — reading the raw flag there compares a block against
* itself forever. Same rule the block renderers apply; none may read the flag alone.
*/
const errorSourceBlockIds = new Set(
edges.filter((edge) => edge.sourceHandle === 'error').map((edge) => edge.source)
)
const errorSourceBlockIds = collectErrorSourceBlockIds(edges)
const blocks: DeployedWorkflowData['blocks'] = {}
for (const [blockId, block] of Object.entries(migratedBlocks)) {
blocks[blockId] =
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -342,6 +342,8 @@ export type WorkflowConnectionSide = (typeof WORKFLOW_CONNECTION_SIDES)[number]
export const WORKFLOW_SOURCE_HANDLE_ID = 'source'
/** The one input handle every block that accepts a connection exposes. */
export const WORKFLOW_TARGET_HANDLE_ID = 'target'
/** The output handle a block's error branch leaves through. */
export const WORKFLOW_ERROR_HANDLE_ID = 'error'
/**
* Side-anchored handle ids (`source-right`, `target-left`, …) briefly existed
Expand Down
Expand Up
@@ -387,6 +389,45 @@ export function normalizeWorkflowEdgeTargetHandle(
return canonical
}
/**
* Collects the ids of blocks an error edge leaves, canonicalizing handles first
* so the set is the same however the edge list was loaded.
*/
export function collectErrorSourceBlockIds(
edges: readonly WorkflowEdgeHandles[] | null | undefined
): Set<string> {
const sources = new Set<string>()
for (const edge of edges || []) {
if (normalizeWorkflowEdgeSourceHandle(edge.sourceHandle) === WORKFLOW_ERROR_HANDLE_ID) {
sources.add(edge.source)
}
}
return sources
}
/**
* Whether a block's error output is on, read from the edges as well as the flag.
*
* An error edge means the port is live whatever the flag says: `setBlockErrorEnabled`
* leaves existing error edges in place, and both block renderers draw the port on
* `errorEnabled || hasErrorConnection`, so a block can sit at `errorEnabled: false`
* with a connected error edge indefinitely. The executor never reads the flag at
* all — the edge alone decides routing — so the two spellings are one state.
*
* Every reader that compares or materializes a block must apply this rule rather
* than the flag alone. Applying it on one side only is what pinned a workflow to
* "needs redeploy" with nothing to deploy: change detection read a backfilled
* `true` against a live `false`, and redeploying snapshotted the live `false` that
* the next read backfilled straight back to `true`.
*/
export function resolveEffectiveErrorEnabled(
block: Pick<BlockState, 'errorEnabled'>,
blockId: string,
errorSourceBlockIds: ReadonlySet<string>
): boolean {
return Boolean(block.errorEnabled) || errorSourceBlockIds.has(blockId)
}
/**
* Canonicalizes a whole edge list, for the readers that bypass
* `loadWorkflowFromNormalizedTables` — deployment-version blobs, run
Expand Down
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.