fix: Resolve race condition between palette fetch and node upload in the app (WB-340) (#51)

* fix: Resolve race condition between palette fetch and node upload in the app (WB-340)

* fix(sdk): add refreshNodesErrorsIfNeeded unit-tests

---------

Co-authored-by: Piotr Blaszczyk <piotr.blaszczyk@synergycodes.com>

Author: szymon-t-sc

.changeset/nodes-palette-race-condition.md

@@ -0,0 +1,5 @@
+---
+'@workflowbuilder/sdk': patch
+---
+
+Simultaneous loading of nodes and the palette can result in a race condition. If nodes are loaded before the palette, new errors (for example, for fields that were not evaluated earlier) are skipped, and the node only shows a “!” indicator when the user selects it.

packages/sdk/src/store/slices/diagram-slice/actions.spec.ts

@@ -0,0 +1,88 @@
+// Pins the WB-340 fix: when nodes load before the palette, they are validated
+// against an empty definition set and their errors are dropped. Once the palette
+// arrives, `refreshNodesErrorsIfNeeded` must re-run validation and repair the
+// stored nodes, while staying a no-op when nothing actually changed.
+import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+
+import { setCustomPaletteNodes } from '../../../data/palette';
+import type { WorkflowBuilderNode } from '../../../node/node-data';
+import { mockNodeDelay } from '../../../utils/validation/get-node-errors.mock';
+import { resetWorkflowStore, useStore } from '../../store';
+import { refreshNodesErrorsIfNeeded } from './actions';
+
+// Mirrors the definition the consumer's palette would supply for `delay` nodes;
+// `description` is required, so a node missing it must surface a validation error.
+const delayDefinition = {
+  type: 'delay',
+  label: 'Delay',
+  description: 'Pause the workflow',
+  icon: 'Timer',
+  defaultPropertiesData: mockNodeDelay.data.properties,
+  schema: {
+    type: 'object',
+    properties: {
+      label: { type: 'string' },
+      description: { type: 'string' },
+      status: { type: 'string' },
+      duration: { type: 'object' },
+      errors: { type: 'array' },
+      type: { type: 'string' },
+    },
+    required: ['label', 'description'],
+  },
+  uischema: { type: 'VerticalLayout', elements: [] },
+};
+
+/** `mockNodeDelay` stripped of its required `description`, with stale (empty) errors. */
+function invalidNode(): WorkflowBuilderNode {
+  const { description: _description, ...properties } = mockNodeDelay.data.properties;
+  return {
+    ...mockNodeDelay,
+    data: { ...mockNodeDelay.data, properties: { ...properties, errors: [] } },
+  };
+}
+
+describe('refreshNodesErrorsIfNeeded', () => {
+  beforeAll(() => {
+    setCustomPaletteNodes([delayDefinition as never]);
+  });
+
+  afterAll(() => {
+    setCustomPaletteNodes(null);
+  });
+
+  beforeEach(() => {
+    resetWorkflowStore();
+  });
+
+  it('repairs errors that were skipped while the palette was unavailable', () => {
+    // Simulates the race: the node was validated before the palette existed, so
+    // its `errors` are empty despite being invalid.
+    useStore.setState({ nodes: [invalidNode()] });
+
+    refreshNodesErrorsIfNeeded();
+
+    const errors = useStore.getState().nodes[0].data.properties.errors;
+    expect(errors).toHaveLength(1);
+    expect(errors).toEqual(expect.arrayContaining([expect.objectContaining({ keyword: 'required' })]));
+  });
+
+  it('leaves the nodes array untouched when no errors change', () => {
+    // `mockNodeDelay` is valid against the definition, so re-validation is a no-op.
+    useStore.setState({ nodes: [mockNodeDelay] });
+    const before = useStore.getState().nodes;
+
+    refreshNodesErrorsIfNeeded();
+
+    // Same reference: the deep-equal guard must skip the `setState` entirely.
+    expect(useStore.getState().nodes).toBe(before);
+  });
+
+  it('does nothing when the store has no nodes', () => {
+    const before = useStore.getState().nodes;
+
+    refreshNodesErrorsIfNeeded();
+
+    expect(useStore.getState().nodes).toBe(before);
+  });
+});

packages/sdk/src/store/slices/diagram-slice/actions.ts

@@ -1,4 +1,6 @@
 // About actions: apps/demo/src/app/store/README.md
+import { isDeepEqual } from 'remeda';
+
 import {
   migrateLegacyHandleIdsOnEdges,
   migrateLegacyHandleIdsOnNodes,
@@ -8,7 +10,7 @@ import type { VariableDefinition } from '../../../features/variables/types';
 import type { LayoutDirection } from '../../../node/common';
 import type { WorkflowBuilderEdge, WorkflowBuilderNode } from '../../../node/node-data';
 import type { IntegrationDataFormat } from '../../../types/integration';
-import { getNodeWithErrors } from '../../../utils/validation/get-node-errors';
+import { getNodeWithErrors, getNodesWithErrors } from '../../../utils/validation/get-node-errors';
 import { useStore } from '../../store';
 import { skipDynamicValuesInEdges, skipDynamicValuesInNodes } from './utils/dynamic-values';
 
@@ -122,6 +124,27 @@ export function getStoreSingleSelected() {
   return selectSingleSelectedElement(state);
 }
 
+/**
+ * Revalidates all nodes in the store and updates their JSON schema validation errors if needed.
+ *
+ * Compares current nodes with revalidated ones and only updates state when errors change,
+ * avoiding unnecessary re-renders.
+ *
+ * @category Store
+ */
+export function refreshNodesErrorsIfNeeded() {
+  const stateNodes = useStore.getState().nodes;
+  const stateNodesWithRefreshedErrors = getNodesWithErrors(stateNodes);
+
+  if (isDeepEqual(stateNodes, stateNodesWithRefreshedErrors)) {
+    return;
+  }
+
+  useStore.setState({
+    nodes: stateNodesWithRefreshedErrors,
+  });
+}
+
 export function saveVariableDefinition(definition: VariableDefinition) {
   useStore.setState((state) => ({
     globalVariables: {

packages/sdk/src/store/slices/palette/palette-slice.ts

@@ -7,6 +7,7 @@ import {
   StatusType,
 } from '../../../node/common';
 import type { GetDiagramState, SetDiagramState } from '../../store';
+import { refreshNodesErrorsIfNeeded } from '../diagram-slice/actions';
 
 export type PaletteState = {
   isSidebarExpanded: boolean;
@@ -40,6 +41,14 @@ export function usePaletteSlice(set: SetDiagramState, get: GetDiagramState): Pal
         data: getPaletteData(),
         fetchDataStatus: StatusType.Success,
       });
+
+      // Defer to the next macrotask: a concurrent node load (its own store
+      // update) may still be pending, and nodes that arrived before the palette
+      // were validated against an empty definition set, so their errors were
+      // dropped. Re-validating after the queue drains repairs them (WB-340).
+      // A macrotask (not `queueMicrotask`) is deliberate — we must yield past
+      // those pending state updates, not just the current microtask queue.
+      setTimeout(() => refreshNodesErrorsIfNeeded(), 0);
     },
     getNodeDefinition: (nodeType) => {
       const { data } = get();

packages/sdk/src/utils/validation/get-node-errors.ts

@@ -42,3 +42,7 @@ export function getNodeWithErrors(node: WorkflowBuilderNode) {
     },
   };
 }
+
+export function getNodesWithErrors(nodes: WorkflowBuilderNode[]): WorkflowBuilderNode[] {
+  return nodes.map(getNodeWithErrors);
+}