fix: graceful degradation — skip unknown step types / transform ops instead of crashing#81
Merged
Merged
Conversation
…f crashing
Cross-app parity with the flowrunner-cli fix. The JS engine had the same
two forward-compat failures:
- flowRunner.js threw on an unknown step type -> the whole run halted.
- transformOps.js silently downgraded an unknown transform op to
base64_decode -> the wrong operation executed.
Both now degrade gracefully (skip-with-machine-readable-warning, continue):
- Unknown step type -> {status:'skipped', unsupported:true} + a user warning.
- Unknown transform op -> recorded in executeTransformOps' warnings[]
(TRANSFORM_OP_UNSUPPORTED marker), surfaced by the transform step; the op's
output var is left unset. normalizeTransformOp is left tolerant because the
editor (flowStepComponents.js) also calls it.
Also fixes the Jest harness: setup.js require()d the vendored UMD
drawflow.min.js, unloadable under the app's "type":"module", so npm test was
fully broken (CI never runs it). Added a CommonJS marker package.json in the
vendored dir. This unblocked the suite (122 passing) and revealed 4
pre-existing flowVisualizer.test.js failures (tracked separately).
Adds 2 unit tests (RED->GREEN). Bible updated (changelog + gotchas).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 4 failures were test-setup/isolation issues, not bugs in
flowVisualizer.js. flowVisualizer.js is unchanged.
- "node moved event triggers layout update callback": the nodeMoved
handler treats the node element's style.left/top as authoritative
(a real Drawflow drag updates both the data model's pos_x/pos_y AND
the DOM element's style.left/top in lock-step). The test only mutated
the data model, so the handler kept reading the stale rendered
position (100,100). Fixed the test to also set the element's
style.left/top, matching a real drag. Assertion unchanged.
- "dragging a node updates flow model layout": same DOM-vs-data-model
issue as above. Additionally, this test created a second
FlowVisualizer without destroying the beforeEach one, leaking that
visualizer's .node-editor-modal into document.body and breaking the
two tests below. Fixed by destroying the old visualizer first and by
moving the DOM element alongside the data model. Assertions unchanged.
- "double-click opens node editor modal": passed document.querySelector
('.node-editor-modal'), which returned a leaked stale modal (display
"none") from the drag test instead of the current visualizer's modal
(display "flex"). Fixed by querying visualizer.nodeEditorModal
directly. Assertions unchanged.
- "add step button triggers add-step callback": same leaked-modal
problem via document.querySelector('.node-editor-add') — it grabbed a
stale, unwired button. Fixed by querying the button within
visualizer.nodeEditorModal. Assertions unchanged.
Full suite: 126 passed, 0 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Cross-app parity with the flowrunner-cli graceful-degradation fix. The FlowRunner UI engine had the same two forward-compatibility failures, so a flow authored with a newer/unknown step type or transform op behaved badly on an older UI.
Fix
flowRunner.js): previously threw → the whole run halted. Now yields{ status: 'skipped', unsupported: true }+ a user-visible warning, and the flow continues.transformOps.js): previously silently downgraded tobase64_decode(ran the wrong operation). Now skipped, recorded inexecuteTransformOps'warnings[](structured entry + aTRANSFORM_OP_UNSUPPORTEDmarker), and surfaced by the transform step; the op's output variable is left unset.normalizeTransformOpis intentionally left tolerant because the editor (flowStepComponents.js) calls it — making it throw would break opening a flow that contains a newer op.Tests + harness
__tests__/setup.jsrequire()d the vendored UMDdrawflow.min.js, which Node treats as ESM under the app's root"type": "module", sonpm testloaded zero tests. (CI never caught it —build.ymlruns onlynpm run dist.) Addedassets/vendor/drawflow/package.json("type": "commonjs") scoping just that vendored dir to CJS; the browser loads Drawflow via a<script>tag, so runtime is unaffected. Suite now runs: 122 passing.Known follow-ups (NOT this PR)
flowVisualizer.test.jsfailures (Drawflow drag / double-click / add-step under jsdom). Verified pre-existing (they fail with this PR's production changes stashed), tracked separately.flowStepComponents.js:911) still rewrites an unknown op tobase64_decodeon open — a separate forward-compat item (seedocs/flowmap-evolution.md).🤖 Generated with Claude Code