fix(ui): temporarily disable frontend undo and redo #3571
Merged
Conversation
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
aglinxinyuan
approved these changes
Jul 18, 2025
core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts
Outdated
Show resolved
Hide resolved
Xiao-zhen-Liu
changed the title
Xiao-zhen-Liu
added a commit
that referenced
this pull request
Oct 7, 2025
This reverts commit ece56ec.
Xiao-zhen-Liu
added a commit
that referenced
this pull request
Oct 10, 2025
…3836) ## Purpose #3571 disabled frontend undo/redo due to an existing bug with the undo/redo manager during shared editing. This PR fixes that bug and re-enables undo/redo. ## Bug with shared editing The bug can be minimally reproduced as follows with two users editing the same workflow (or two tabs opened by the same user): 1. User A deletes a link E from operator X to Y on the canvas, 2. User B deletes operator Y. 3. User A clicks "undo", and the workflow reaches an erroneous state, where there is a link E that connects to an operator Y that no longer exists. Note E exists in the frontend data but is not visible on the UI. The following gif shows this process.  ## Shared-editing Architecture Shared editing (#1674) is achieved by letting the frontend rely on data structures from yjs (a CRDT library) as its data model, as any manipulation to these data structures can be propagated to other users with automatic conflict-resolution. There are two layers of data on each user's Texera frontend, one being the UI data (jointjs), and the other being this shared "Y data". The two layers in each user's UI are synched by our application code, and the Y data between users of a shared-editing sessions are kept in sync with automatic conflict resolution by relying on yjs. The following diagram shows what happens when a user adds a link and how the other user sees this change in real-time.  Yjs's CRDT guarantees the eventual **consistency** of this underlying data model among concurrent editors, i.e., it makes sure this data model is correctly synced in each editor's frontend. ## The core problem Yjs does not offer a "graph" data structure, and currently in Texera, the shared data structures for operators and links are two separate `Map`s: - `operatorIDMap`: `operatorID`->`Operator` - `operatorLinkMap`: `linkID`-> `OperatorLink` There is an application-specific "referential constraint" in Texera's frontend that "a link must connect to an operator that exists", and this kind of sanity checking on the data is not the concern of CRDT. It can only be enforced by the application (i.e., ourselves). Ideally, before making any changes to the shared data model, we should do sanity checking and reject changes that violate our application-specific constraints. As shown below, in each user's frontend, there are 3 paths where the shared data model can be modified.  **Path 1**: The first is path includes those changes initiated by a user's UI actions (e.g., add a link on the UI). For this path, we do have existing sanity checking logic: ``` public addLink(link: OperatorLink): void { this.assertLinkNotExists(link); this.assertLinkIsValid(link); this.sharedModel.operatorLinkMap.set(link.linkID, link); } ``` **Path 2**: Another path is undo/redo, which is purely managed by an `UndoManager`, also offered by Yjs. This module is local to each user's frontend, and it automatically tracks local changes to the shared data model. When a user clicks "undo", `UndoManager` directly applies changes to the shared data model. **The core of the problem is there is no sanity checking on this path.** **Path 3**: The third path is remote changes from another collaborator. There is also no sanity checking on this path, but the correctness of such changes depends on whether the change was sanity-checked on the collaborator's side (i.e., if it is a UI change from User A, the propagated change to User B's frontend would be sanity-checked; if it is a undo change, however, the propagated changed to User B would not be sanity-checked and could cause issues.) ## Cause of the bug The following diagram shows how the bug happens from the perspective of the shared model.  When user A clicks "Undo" after 2), the `UndoManager` simply applies the reverse-operation of "Delete E", and add the link `E` to `operatorLinkMap `. As there is no sanity checking during this process, this operation succeeds, and the shared model reaches a state that violates the constraint. ## Solution Unfortunately, due to the limitations of Yjs's APIs, it is not possible to add sanity checking to Path 2 or 3 **before** a change is applied, as an undo/redo operation on the `UndoManager`'s stack is not exposed as a meaningful action (i.e., there is no way to tell that an action to be applied to the shared model is an `addLink` if it is an undo operation). Nevertheless, we can react to a change to the shared model that is initiated from Path 2 or Path 3 after the change has been applied, and add sanity checking logic there to "repair" unsanitary changes. This places (`SharedModelChangeHandler`) is exactly where we sync the changes from the shared model to the UI: any changes to the shared model not initiated by the UI (i.e., changes from the `UndoManager` or remote changes by other users) go through this place, and such changes are parsed as meaningful changes such as "add a link", "delete an operator", etc.  Currently, the only sanity checking needed is to check if a newly added link connects to operator / ports that exist and that it is not a duplicate link. We add such checking logic in `SharedModelChangeHandler`, and revert unsanitary operations before it is reflected on the UI. ## Demo The following gif shows the experience after the fix. When unsanitary actions caused by undo happens, it would fail and we log it in the console. The workflow JSON no longer gets damaged.  --------- Co-authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
We observed in #3553 that undo/redo can occasionally cause a workflow state on the frontend to be invalid. As we need to spend more time to investigate the issue, we decide to temporarily disable undo/redo until we fix the issue.