[ZEPPELIN-6563] Collaborative mode can silently desynchronize paragraph text, and a later commit overwrites the server copy

[xhaktm00]

What is this PR for?

In collaborative mode paragraph text is synchronised with diff-match-patch, and both the receiving client (paragraph-base.ts) and the server (NotebookService.java) use only element [0] of patch_apply, discarding the per-hunk success array. A patch that cannot be applied does not throw, and it fails in two different shapes:

• dropped — with no matching context, patch_apply returns the original text with [false], so the sender's edit silently disappears on that client
• misapplied — fuzzy matching can place a hunk at the wrong offset and still report [true]. If one user changes bravo to bravo! while another has deleted that line, the second client ends up with alpha\n!charlie\n and a success flag

The second shape is why inspecting the success array is not enough, and the ticket already ruled out both that approach and Match_Threshold = 0.

A diverged paragraph does not stay local. Focusing and blurring calls commitParagraph, which sends the client's whole paragraph, and the server stores it through p.setText(text) without comparing it to what it holds, then broadcasts it to every connection. Whichever client commits first makes its own text authoritative for everyone.

This PR keeps the existing patch mechanism and makes divergence detectable instead:

• Detection — senders attach checksums of their text before and after the patch. A receiver that started from the same text but did not reach the same result knows it diverged. When the base checksums differ the receiver is simply editing concurrently, so no verdict is made and the current behaviour is kept; this is what keeps ordinary concurrent editing from being flagged. Sending the text itself would defeat the purpose of patching — the two checksums add about 37 bytes to a ~160 byte patch message.
• Recovery — a diverged receiver asks for that one paragraph again through a new GET_PARAGRAPH message. The ticket rejected refetching the note because it recreates every editor while the user is typing, so recovery is per paragraph. broadcastParagraph already sends a single paragraph, but a client had no way to ask for one; the response reuses the existing OP.PARAGRAPH handler.
• Containment — a commit carries the checksum of the text the client believed the server held. When it does not match, the server keeps its own copy and sends it back rather than storing text that may be diverged.

Both fields are optional: clients that do not send them behave exactly as before.

Two things I deliberately left out of scope, and some type-level changes I had to include:

• The server-side patchApply(...)[0] is unchanged. The server holds a single authoritative copy and applies every patch in order, so it does not diverge the way a client with local edits does, and the commit check backs it up. I can add a warning there in a follow-up if you would prefer it for diagnostics.
• The classic UI is unchanged. zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js has the same line, but with the new UI as default the classic one is served under /classic and has to be opened deliberately. Since the fields are optional it keeps working unchanged, and mixing the two UIs is safe apart from the classic client losing the check. Would you prefer I cover it here, or file a follow-up?
• Two type-level changes came along with this. PatchParagraphSend and PatchParagraphReceived were mapped to the opposite directions in message-data-type-map.interface.ts, which left the receiving side typed without noteId or the new fields, so they are swapped to match what is actually sent. And after rebasing onto the merged ZEPPELIN-6582, the checksums are computed inside its makeParagraphPatch() rather than inline in the component; reaching textChecksum from that spec needs vitest to resolve @zeppelin/sdk, so vitest.shell.config.mts maps that one alias to the SDK source. Only the SDK is mapped, since the others pull in the JIT compiler and monaco. Happy to split either out if you would rather review them separately.

Finally, this does not make the sync conflict-free. diff-match-patch locates a hunk by matching surrounding context, so a concurrent edit that removes that context can still make a patch land in the wrong place. Editors like Google Docs avoid this class of bug with Operational Transformation or CRDTs, where each edit carries an explicit position the server transforms against concurrent operations. Replacing Zeppelin's sync engine would be far larger than this ticket, so this PR takes the narrower path the ticket points at: make divergence detectable, and stop one client's diverged text from becoming everyone's.

What type of PR is it?

Bug Fix

Todos

• Send before/after checksums with each patch
• Verify the patched result on the receiver and request the paragraph again when it diverged
• Add GET_PARAGRAPH so a single paragraph can be resent
• Reject a commit whose base no longer matches the server text
• Add unit tests for the commit check

What is the Jira issue?

• ZEPPELIN-6563

How should this be tested?

New test NotebookServiceTest#testUpdateParagraphChecksIsBasedOnCurrentServerText covers the commit check: a commit based on the current server text is stored, one based on text the server no longer holds is rejected and leaves the server copy untouched, and a commit without a checksum keeps the previous behaviour.

./mvnw -pl zeppelin-server --am test -Dtest=NotebookServiceTest -DfailIfNoTests=false

Result: Tests run: 10, Failures: 0, Errors: 0. --am matters here: this adds GET_PARAGRAPH to Message.OP in zeppelin-common, so building zeppelin-server alone compiles the tests against a stale copy and fails on an unrelated symbol. If a local Zeppelin server is running it holds the Lucene index lock and every test errors out in setUp, so stop it first or point ZEPPELIN_SEARCH_INDEX_PATH at a scratch directory.

The checksum uses the same algorithm as String.hashCode() so both sides compute the same value; I verified Java and TypeScript agree on empty strings, newlines, non-ASCII text, emoji (surrogate pairs) and long input.

cd zeppelin-web-angular && npm run test:shell

Result: Tests 31 passed (31), including paragraph-patch.spec.ts covering the checksums the sender attaches.

Manual: open the same note in two browser sessions and edit the same paragraph concurrently, with one session deleting a line while the other edits it. I confirmed the built bundle ships the new fields and that ordinary collaborative sync still works, but I could not reproduce the divergence by hand — on a local server the patch round trip is fast enough that the window never opens. This matches the ticket's note that existing e2e coverage does not exercise failed or misapplied patches, which is why the commit check is covered by unit tests instead. Rejecting stale commit of paragraph in the server log marks a refused commit.

Screenshots (if appropriate)

N/A

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No — the new fields are optional, and the new op is only sent after a detected divergence
• Does this needs documentation? No

[voidmatcha]

Thanks for working on this. I found two correctness cases worth addressing before merge:

1. In personalized mode, GET_NOTE returns the user-specific note, but updateParagraph() checks baseChecksum against the master paragraph before selecting the user paragraph. Collaborative mode can also be active on a personalized note. A valid user-specific commit can therefore be rejected, and GET_PARAGRAPH currently returns the master paragraph for recovery. Could the checksum guard and recovery response use the paragraph version the client is editing, or explicitly handle this combined mode, with a regression test?

2. When a patch diverges, the client requests GET_PARAGRAPH. If another local edit occurs before the reply, updateAllScopeTexts() can replace paragraph.text while leaving originalText at a different version. That risks losing the later edit or building the next patch against the wrong base. Could recovery preserve edits made after the request or ignore a stale reply, with a test for this ordering?

This branch also currently conflicts with master, so please rebase once these cases are addressed.

[xhaktm00]

@voidmatcha Thank you for the careful review. Both cases were real, and I have addressed them.

1. Personalized mode checked the wrong paragraph

• Cause: GET_NOTE answers with note.getUserNote(user), but in updateParagraph() the switch to the user copy happened after the checksum guard, so baseChecksum was compared against the master text. Once they diverge, a valid non-owner commit is rejected and answered with the master paragraph, leaving no route back. getParagraph() had the same gap.
• Fix: both now resolve the paragraph the client edits before using it. getUserParagraph() clones the master when no copy exists, so a first edit still compares equal.

2. Recovery could drop an edit made during the request

• The part you described was already fixed on master — updateAllScopeTexts now moves the three text values together, and msgId tracking was added.
• Cause: what remained was my own change. The GET_PARAGRAPH reply did not echo msgId, so it never entered that tracking:

acknowledged = false   (no msgId → no pending save)
hasLocalEdit = true
originalText === newText = false
→ both guards false → dirtyText = undefined

• Fix: the reply now echoes msgId like the other paragraph broadcasts, and the client tracks recovery requests in pendingParagraphFetches, separate from saves. The later edit stays in dirtyText while the editor re-bases on the server text — covering both options you offered.

Tests

./mvnw test -pl zeppelin-server --am -Dtest=NotebookServiceTest
cd zeppelin-web-angular && npm run test:shell

16 server and 251 frontend tests pass. Reverting each fix fails the matching test:

expected: <user1 edit> but was: <user1 text>
expected: <user1 text> but was: <master text>
AssertionError: expected undefined to be 'edit typed while waiting'