fix: Prevent corruption of workflow data due to checkState during graph loading#9531
Conversation
🎭 Playwright: ✅ 558 passed, 0 failed · 1 flaky📊 Browser Reports
|
📝 WalkthroughWalkthroughThe changes introduce a loading state guard mechanism by adding a new Changes
Sequence DiagramsequenceDiagram
participant App as src/scripts/app.ts
participant CT as ChangeTracker
participant Store as appModeStore
App->>CT: Set isLoadingGraph = true
App->>App: Begin graph loading sequence
App->>App: Configure layout & load graph data
activate App
Store->>Store: Data persistence triggered
Store->>CT: Check isLoadingGraph?
alt Loading in progress
Store->>Store: Skip data save (early exit)
end
deactivate App
App->>App: Scale view, process nodes/widgets
App->>App: Invoke extensions, emit telemetry
App->>CT: Set isLoadingGraph = false (finally)
CT->>CT: checkState() now allowed to serialize
Store->>Store: Data persistence resumes normally
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook: ✅ Built — View Storybook |
📦 Bundle: 4.57 MB gzip 🟢 -540 BDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • 🟢 -33 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 950 kB (baseline 950 kB) • 🟢 -45 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 22 added / 22 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 77.6 kB (baseline 77.6 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 56.5 kB (baseline 56.5 kB) • ⚪ 0 BReusable component library chunks
Status: 13 added / 13 removed Data & Services — 2.77 MB (baseline 2.76 MB) • 🔴 +9.27 kBStores, services, APIs, and repositories
Status: 15 added / 15 removed Utilities & Hooks — 57.3 kB (baseline 57.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 15 added / 15 removed Vendor & Third-Party — 8.87 MB (baseline 8.87 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 5 added / 5 removed Other — 8.04 MB (baseline 8.05 MB) • 🟢 -9.48 kBBundles that do not match a named category
Status: 120 added / 121 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-03-07T20:13:24.701Z",
"gitSha": "3fba0aa036ad0025dfac8bc33f5663fca4621487",
"branch": "pysssss/fix-workflow-corruption",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2025.9340000000066,
"styleRecalcs": 123,
"styleRecalcDurationMs": 20.651000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 396.93999999999994,
"heapDeltaBytes": -3151432
},
{
"name": "canvas-idle",
"durationMs": 2026.2910000000147,
"styleRecalcs": 123,
"styleRecalcDurationMs": 20.141,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 396.89000000000004,
"heapDeltaBytes": -3558688
},
{
"name": "canvas-idle",
"durationMs": 2039.4699999999943,
"styleRecalcs": 124,
"styleRecalcDurationMs": 19.151,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 384.91700000000003,
"heapDeltaBytes": -2879192
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2064.9590000000444,
"styleRecalcs": 186,
"styleRecalcDurationMs": 52.45300000000001,
"layouts": 12,
"layoutDurationMs": 3.4150000000000005,
"taskDurationMs": 1079.267,
"heapDeltaBytes": -3112336
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1829.3600000000083,
"styleRecalcs": 166,
"styleRecalcDurationMs": 43.889,
"layouts": 12,
"layoutDurationMs": 3.0709999999999997,
"taskDurationMs": 810.211,
"heapDeltaBytes": -3445028
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1808.4779999999796,
"styleRecalcs": 164,
"styleRecalcDurationMs": 41.802,
"layouts": 12,
"layoutDurationMs": 2.9859999999999998,
"taskDurationMs": 770.4499999999999,
"heapDeltaBytes": -3032328
},
{
"name": "dom-widget-clipping",
"durationMs": 611.8000000000166,
"styleRecalcs": 45,
"styleRecalcDurationMs": 14.440000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 405.10900000000004,
"heapDeltaBytes": 7422096
},
{
"name": "dom-widget-clipping",
"durationMs": 603.8730000000214,
"styleRecalcs": 44,
"styleRecalcDurationMs": 12.411999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 355.716,
"heapDeltaBytes": 7700456
},
{
"name": "dom-widget-clipping",
"durationMs": 557.7329999999847,
"styleRecalcs": 41,
"styleRecalcDurationMs": 11.817999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 345.613,
"heapDeltaBytes": 7527712
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 623.2530000000338,
"styleRecalcs": 77,
"styleRecalcDurationMs": 18.654999999999998,
"layouts": 1,
"layoutDurationMs": 0.335,
"taskDurationMs": 442.524,
"heapDeltaBytes": -8218128
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 627.5339999999687,
"styleRecalcs": 75,
"styleRecalcDurationMs": 16.075,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 430.449,
"heapDeltaBytes": 15086984
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 574.8179999999934,
"styleRecalcs": 72,
"styleRecalcDurationMs": 14.828000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 415.3310000000001,
"heapDeltaBytes": -8934448
},
{
"name": "subgraph-idle",
"durationMs": 2012.5250000000392,
"styleRecalcs": 123,
"styleRecalcDurationMs": 23.011,
"layouts": 1,
"layoutDurationMs": 0.21199999999999994,
"taskDurationMs": 387.322,
"heapDeltaBytes": -3998788
},
{
"name": "subgraph-idle",
"durationMs": 2007.772999999986,
"styleRecalcs": 121,
"styleRecalcDurationMs": 18.948,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 379.024,
"heapDeltaBytes": -3402048
},
{
"name": "subgraph-idle",
"durationMs": 1993.7810000000127,
"styleRecalcs": 120,
"styleRecalcDurationMs": 18.643,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 376.84,
"heapDeltaBytes": -3625916
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1997.191999999984,
"styleRecalcs": 172,
"styleRecalcDurationMs": 52.574000000000005,
"layouts": 16,
"layoutDurationMs": 4.523,
"taskDurationMs": 980.4750000000001,
"heapDeltaBytes": -5096852
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1696.883000000014,
"styleRecalcs": 155,
"styleRecalcDurationMs": 43.376,
"layouts": 16,
"layoutDurationMs": 4.2589999999999995,
"taskDurationMs": 723.5409999999999,
"heapDeltaBytes": -5794872
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1681.3339999999926,
"styleRecalcs": 153,
"styleRecalcDurationMs": 43.152,
"layouts": 16,
"layoutDurationMs": 4.472,
"taskDurationMs": 716.929,
"heapDeltaBytes": -5660824
}
]
} |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/scripts/app.ts (1)
1310-1310: Consider counter-based approach for robustness against nested loads.The codebase already uses
configuringGraphLevel(a counter) for similar purposes. A counter-based approach would be more robust ifloadGraphDatawere ever called recursively (e.g., from an extension hook). Currently this seems unlikely given the code structure, so the boolean is acceptable for this fix.♻️ Optional: Counter-based approach for future-proofing
export class ChangeTracker { static MAX_HISTORY = 50 - static isLoadingGraph = false + private static loadingGraphLevel = 0 + static get isLoadingGraph(): boolean { + return this.loadingGraphLevel > 0 + } + static enterLoadingGraph(): void { + this.loadingGraphLevel++ + } + static exitLoadingGraph(): void { + this.loadingGraphLevel-- + }Then in
app.ts:- ChangeTracker.isLoadingGraph = true + ChangeTracker.enterLoadingGraph() try { // ... } finally { - ChangeTracker.isLoadingGraph = false + ChangeTracker.exitLoadingGraph() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/app.ts` at line 1310, Replace the boolean flag usage with a counter to guard nested loads: instead of setting ChangeTracker.isLoadingGraph = true/false in loadGraphData, increment ChangeTracker.configuringGraphLevel (or add that numeric property if missing) at the start of the load and decrement it in a finally block at the end; update any checks that read ChangeTracker.isLoadingGraph to instead test ChangeTracker.configuringGraphLevel > 0 so recursive/nested calls are correctly handled. Ensure increments/decrements are paired even on errors by performing the decrement in the finally path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/scripts/app.ts`:
- Line 1310: Replace the boolean flag usage with a counter to guard nested
loads: instead of setting ChangeTracker.isLoadingGraph = true/false in
loadGraphData, increment ChangeTracker.configuringGraphLevel (or add that
numeric property if missing) at the start of the load and decrement it in a
finally block at the end; update any checks that read
ChangeTracker.isLoadingGraph to instead test ChangeTracker.configuringGraphLevel
> 0 so recursive/nested calls are correctly handled. Ensure
increments/decrements are paired even on errors by performing the decrement in
the finally path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e5afc2f6-d820-42e2-b71e-fd02b5b84a73
📒 Files selected for processing (3)
src/scripts/app.tssrc/scripts/changeTracker.tssrc/stores/appModeStore.ts
|
@coderabbitai Log an issue to write a unit test to prevent a regression for this issue. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 57
🧠 Learnings used |
|
@coderabbitai Log an issue to see if we can rearchitect this to avoid getting into the desynced workflow/graph data state completley |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 57
🧠 Learnings used |
AustinMroz
left a comment
There was a problem hiding this comment.
PRs like this make me very happy for git diff -w/(hide whitespace)
…ph loading (#9531) ## Summary During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` ## Changes - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io)
|
@pythongosssss Successfully backported to #9537 |
… checkState during graph loading (#9537) Backport of #9531 to `cloud/1.41` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9537-backport-cloud-1-41-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-g-31c6d73d3650810a9963e85b18084e31) by [Unito](https://www.unito.io) Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com>
…ph loading (#9531) ## Summary During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` ## Changes - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io)
|
…ph loading (#9531) ## Summary During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` ## Changes - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io)
|
…ph loading (#9531) ## Summary During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` ## Changes - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io)
…checkState during graph loading (#9531) (#9561) Backport of #9531 to core/1.41. Cherry-pick of merge commit ec129de applied cleanly. **Original PR:** #9531 **Pipeline ticket:** 15e1f241-efaa-4fe5-88ca-4ccc7bfb3345 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9561-backport-core-1-41-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-gr-31d6d73d365081aabc05c0409ee49615) by [Unito](https://www.unito.io) Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com>
|
|
|
…ph loading (#9531) During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow. Recreation steps: * Open 2-3 workflows * Enter builder mode > select step * Select some different inputs on each * Quickly tap the shift key (this triggers checkState) while switching tabs * After a while, you'll see the wrong inputs on the workflows Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs: ``` window.app.registerExtension({ name: 'bad', async afterConfigureGraph() { window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState() } }) ``` - **What**: - Add loading graph flag - Prevent checkState calls while loading - Prevent app mode data sync while loading ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025) by [Unito](https://www.unito.io)
…checkState during graph loading (#9531) (#9579) Backport of #9531 to core/1.40. Critical data corruption fix. Conflicts resolved: restructured try/catch in app.ts to wrap with ChangeTracker.isLoadingGraph. Removed appModeStore.ts (app mode not on 1.40). **Original PR:** #9531 **Pipeline ticket:** 15e1f241-efaa-4fe5-88ca-4ccc7bfb3345 Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com>
Coverage for 10 bug gaps identified during deep persistence audit: CRITICAL: - PR #9531: Workflow data corruption from checkState during graph loading (pythongosssss fix — had ZERO tests, now covered with 2 tests) - PR #9533: Desynced workflow/graph state during rapid tab switching MEDIUM: - PR #9380: Node output previews lost on tab switch - 44bb6f1: Canvas not cleared before loading new workflow - PR #7648: Widget values lost on graph change - PR #9694: API format workflows fail with missing nodes - PR #8259: Middle-click paste duplicates workflow - PR #8715: Transient blob: URLs in serialization LOW: - PR #8963: Locale change breaks workflows - Splitter panel size drift All tests use Vue nodes with new menu enabled. Each test documents which PR/commit it regresses and reproduces the exact user scenario. Part of: Test Coverage Q2 Overhaul (REG-01)
Summary
During workflow loading, the workflow data & active workflow object can be out of sync, meaning any checkState calls will overwrite data into the wrong workflow.
Recreation steps:
Alternatively, register an extension that guarantees to call checkState during the bad phase, run this in browser devtools and switch tabs:
Changes
┆Issue is synchronized with this Notion page by Unito