From 8746c9c0df98eee6b75d85ef10502fbdd77a5d05 Mon Sep 17 00:00:00 2001 From: nubs Date: Sun, 7 Jun 2026 20:35:35 +0000 Subject: [PATCH] fix(documents): discard pending AI diff before switching active doc (#2484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The document editor stores the AI-edit diff state (_diffModeActive, _diffOldContent, _diffNewContent, _diffChunks) as a module-global singleton bound to whatever document was active when the diff opened, and every document shares one #doc-editor-textarea. When the active document is switched while an unapproved diff is open, the stale diff must be discarded first or a later exitDiffMode (tab switch / Accept-Reject-All) flushes the old document's content into the new active document and overwrites it (issue #2467). Guard both paths that switch the active document for an AI update, while activeDocId still points at the previously-active doc: - handleDocUpdate(): a doc_update targeting a different document. - streamDocOpen(): the AI streaming a NEW document — this runs first on that path, so a guard only in handleDocUpdate would fire too late and still overwrite the streamed document. Both reuse the exact `if (_diffModeActive) exitDiffMode(true);` guard switchToDoc() and enterDiffMode() already use. Fixes #2467 --- static/js/document.js | 18 +++++ ...test_document_diff_discard_on_update_js.py | 77 +++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/test_document_diff_discard_on_update_js.py diff --git a/static/js/document.js b/static/js/document.js index e2d2e7608..86ecf2880 100644 --- a/static/js/document.js +++ b/static/js/document.js @@ -8978,6 +8978,14 @@ import * as Modals from './modalManager.js'; /** Open the document panel immediately for a doc being streamed in */ export function streamDocOpen(title, language) { + // Discard any pending AI-edit diff before this stream changes the active + // document. When the AI streams a NEW document while an unapproved diff is + // open on the current one, streamDocOpen reassigns activeDocId below; if the + // stale diff isn't cleared first, a later exitDiffMode applies the old doc's + // content to the new one and overwrites it (issue #2467). activeDocId still + // points at the previously-active doc here, so exitDiffMode(true) restores + // and saves THAT doc — same guard handleDocUpdate/switchToDoc use. + if (_diffModeActive) exitDiffMode(true); // If already streaming a doc, reuse it (don't create a second temp doc) if (_streamDocId && docs.has(_streamDocId)) { const existing = docs.get(_streamDocId); @@ -9216,6 +9224,16 @@ import * as Modals from './modalManager.js'; /** Handle SSE doc_update event from AI */ export function handleDocUpdate(data) { const streamingId = streamDocFinalize(); + // Discard any pending AI-edit diff before this update changes the active + // document. The diff state (_diffModeActive/_diffOldContent/...) is a + // module-global singleton bound to whatever doc was active when the diff + // opened; if we switch documents without clearing it, a later tab switch or + // Accept/Reject-All flushes the stale diff's content into the now-active + // doc and silently overwrites it (issue #2467). activeDocId still points at + // the previously-active doc here, so exitDiffMode(true) restores and saves + // THAT doc before we reassign activeDocId below — mirroring switchToDoc() + // and enterDiffMode(). + if (_diffModeActive) exitDiffMode(true); let docId = data.doc_id; const newContent = data.content || ''; diff --git a/tests/test_document_diff_discard_on_update_js.py b/tests/test_document_diff_discard_on_update_js.py new file mode 100644 index 000000000..eb2ed05b0 --- /dev/null +++ b/tests/test_document_diff_discard_on_update_js.py @@ -0,0 +1,77 @@ +"""Regression guard for issue #2467 — cross-document overwrite via a stale AI-edit diff. + +document.js keeps the AI-edit diff state (``_diffModeActive`` / ``_diffOldContent`` / +``_diffNewContent`` / ``_diffChunks``) as a module-global singleton bound to whatever +document was active when the diff opened. ``handleDocUpdate()`` switches the active +document (``activeDocId``) whenever an AI update targets a different doc. If a pending +diff is not discarded first, a later tab switch (``switchToDoc`` → ``exitDiffMode(true)``) +or Accept/Reject-All flushes the stale diff's content into the now-active document and +silently overwrites it. + +The fix discards any pending diff while ``activeDocId`` still points at the +previously-active doc, mirroring the guard ``switchToDoc()`` and ``enterDiffMode()`` +already use. It must run in BOTH places that switch the active document for an AI +update: ``handleDocUpdate()`` and ``streamDocOpen()``. The streamed path matters most — +when the AI creates a NEW document (the issue's own repro), ``streamDocOpen`` reassigns +``activeDocId`` first, so a guard only in ``handleDocUpdate`` would fire too late and +still overwrite the new doc. Kept as a static source check because document.js is +browser-coupled and not importable in pytest. +""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +DOC_JS = (ROOT / "static/js/document.js").read_text() + +GUARD = "if (_diffModeActive) exitDiffMode(true);" + + +def _function_body(src: str, signature: str) -> str: + """Return the full text of a JS function, brace-matched from its signature.""" + start = src.index(signature) + depth = 0 + i = src.index("{", start) + while i < len(src): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + i += 1 + raise AssertionError(f"unbalanced braces after {signature!r}") + + +HANDLE_DOC_UPDATE = _function_body(DOC_JS, "export function handleDocUpdate(data)") +STREAM_DOC_OPEN = _function_body(DOC_JS, "export function streamDocOpen(title, language)") + + +def test_handle_doc_update_discards_pending_diff(): + # A new AI update on a different document must not leave a stale diff bound + # to the old doc, or a later tab switch / Accept-All overwrites the wrong doc. + assert GUARD in HANDLE_DOC_UPDATE + + +def test_diff_discard_runs_before_active_doc_is_switched(): + # The discard must run while activeDocId still points at the previously + # active doc, so exitDiffMode(true) restores and saves THAT doc — not the new + # one. Any activeDocId reassignment inside handleDocUpdate must come after it. + guard_at = HANDLE_DOC_UPDATE.index(GUARD) + reassign_at = HANDLE_DOC_UPDATE.index("activeDocId = docId;") + assert guard_at < reassign_at + + +def test_stream_doc_open_discards_pending_diff_before_switching(): + # The AI-creates-a-new-document path switches activeDocId inside + # streamDocOpen (before any doc_update reaches handleDocUpdate), so the guard + # must be here too — and before streamDocOpen reassigns activeDocId, or the + # streamed new doc gets overwritten by the stale diff (the issue's own repro). + assert GUARD in STREAM_DOC_OPEN + assert STREAM_DOC_OPEN.index(GUARD) < STREAM_DOC_OPEN.index("activeDocId = docId;") + + +def test_diff_discard_reuses_the_existing_idiom(): + # Sanity: this exact guard is the established pattern (switchToDoc, + # enterDiffMode, handleDocUpdate, streamDocOpen, …) — the fix reuses it + # rather than inventing a new mechanism. + assert DOC_JS.count(GUARD) >= 5