[ZEPPELIN-6683] Prevent stale notebook WebSocket replies from mutating the active note

[gyowoo1113]

What is this PR for?

This PR prevents stale notebook WebSocket replies from mutating the currently active note.

GET_INTERPRETER_BINDINGS, SAVE_INTERPRETER_BINDINGS, LIST_REVISION_HISTORY, checkpoint creation, and SET_NOTE_REVISION previously returned replies without preserving the originating request identity. The SDK receive() path also exposed only message.data, so notebook listeners could not determine which note a late reply belonged to.

This PR preserves the originating msgId on the relevant server reply paths, exposes the full WebSocket envelope through the SDK, and records the note context for pending requests on the client. Notebook handlers now reject replies with missing or unknown request identities, as well as replies whose request note no longer matches the active route.

The existing operation names are unchanged, and the change is limited to interpreter-binding and revision-related reply paths.

What type of PR is it?

Bug Fix

Todos

• Preserve msgId on INTERPRETER_BINDINGS responses
• Preserve msgId on LIST_REVISION_HISTORY responses, including checkpoint replies
• Preserve msgId on SET_NOTE_REVISION responses
• Expose WebSocket reply envelopes through the SDK
• Track pending request note context and reject stale, missing, or unknown replies
• Add server tests for all affected response paths
• Add frontend tests for stale reply handling and valid same-note handling

What is the Jira issue?

[ZEPPELIN-6683]

How should this be tested?

Frontend focused tests (from zeppelin-web-angular):

npm run test:shell -- \
  projects/zeppelin-sdk/src/message.spec.ts \
  src/app/core/message-listener/message-listener.spec.ts \
  src/app/pages/workspace/notebook/notebook.component.spec.ts

WebSocket contract check:

npm run check:websocket-contract

Server focused test:

./mvnw -pl zeppelin-server -Dtest=NotebookServerTest test

All checks above pass successfully.

Screenshots (if appropriate)

N/A

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No
• Does this needs documentation? No

[tbonelee]

consumePendingNoteRequest deletes the entry as soon as it reads it, so a single reply can only ever be handled by one listener. Zeppelin's receive path filters received$ by OP and fans out to every subscriber, which does not match that assumption.

• OP.PARAGRAPH is subscribed by as many components as ParagraphBase has live instances, which is one per rendered paragraph (paragraph-base.ts:148, action-bar.component.ts:138).
• The server attaches msgId to broadcasts as well (NotebookServer.java:756, :768, :783). msgId is a correlation id for pairing a request with its reply, not a token that the first reader claims.

This stays hidden today because each of the three OPs has exactly one listener. But the rejection path returns without logging, so if a second listener is ever added, the only symptom will be "the view does not update" with nothing to trace it back to.

As an alternative, I would suggest carrying noteId in the reply payload. All five response sites already have noteId in hand, so the server-side change is about the same size as the current one.

new Message(OP.INTERPRETER_BINDINGS).put("noteId", noteId).put("interpreterBindings", settingList)

if (data.noteId !== noteId) {
  return;
}

That leaves no client-side state to maintain, so the cleanup question for the pending map and the implicit contract that every caller must go through MessageService both disappear with it. It also matches how ParagraphBase already tells its own paragraph apart with newPara.id !== oldPara.id.

One thing to settle first: ZEPPELIN-6683 scopes this as preserving a stable request identity, so moving in this direction means adjusting the issue scope. What do you think?

[gyowoo1113]

Thanks, that makes sense. I agree that carrying noteId in the reply payload fits the fan-out behavior better, and that the pending msgId -> noteId state should be removed.

I think msgId still has a separate role as a stable request identity for correlating a reply with the request that caused it, independently of using noteId to identify which note should handle the reply.

So my preference would be to remove the pending-state/consume logic, add noteId to these reply payloads, but keep the envelope support and the server-side withMsgId propagation for request/reply correlation.

Do you think it makes sense to keep that part, or would you prefer to remove the envelope/msgId propagation from the scope of ZEPPELIN-6683 as well? I'm happy to adjust it either way.

[tbonelee]

Agreed. Splitting the two things you asked about:

The server-side withMsgId propagation I would suggest dropping. Once the handlers decide on noteId, nothing reads msgId on those five reply paths, and a contract with no consumer can only be tested as "the value comes back unchanged". Better to add it together with the consumer, whenever a feature actually needs request/reply correlation.

I would keep the envelope support, and give it a consumer in this PR. There is already a workaround it can replace: interceptReceived in message.service.ts:53-59 copies the envelope's msgId into the payload.

if (received.op === OP.PARAGRAPH_ADDED && received.data && received.msgId) {
  (received.data as MessageReceiveDataTypeMap[OP.PARAGRAPH_ADDED]).msgId = received.msgId;
}

Handlers cannot reach the envelope, so ParagraphAdded declares msgId?: string (message-notebook.interface.ts:170) and the value is moved there on receive. receiveEnvelope can replace that outright:

• switch addParagraph (notebook.component.ts:149) to @MessageEnvelopeListener(OP.PARAGRAPH_ADDED) and read message.msgId
• drop the copy block in interceptReceived
• remove msgId?: string from ParagraphAdded

So: drop the server-side withMsgId, keep the envelope API and land it together with that cleanup. Let me know if you see it differently.

[gyowoo1113]

Updated based on feedback:

• Removed the server-side withMsgId propagation from the notebook reply paths.
• Added noteId to the relevant reply payloads and used it to ignore stale replies on the client.
• Kept the envelope API and switched PARAGRAPH_ADDED to MessageEnvelopeListener, so it now reads msgId directly from the envelope.
• Removed the PARAGRAPH_ADDED payload msgId workaround from interceptReceived.
• Added/updated tests for the noteId guards and the envelope msgId consumer.

Thanks for the feedback.