fix(#244): two HIGH data-loss bugs — lossless markdown export + store-side empty-guard #248
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#248
Reference in New Issue
Block a user
Delete Branch "fix/244-dataloss-bugs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Долг #244 Часть A — два HIGH-бага потери данных
Закрывает Часть A issue #244: два подтверждённых бага потери данных, которые PR #230 осознанно оставил незафиксированными (только характеризующие
it.fails/it.failing). Теперь это настоящие фиксы, тесты зелёные.mdrt-2 — экспорт в Markdown молча выкидывал кастом-узлы
turndown.utils.ts: добавлены 4 правила (pageBreak,transclusionReference,mention,status) + предобработка. Две причины: (1) blank-node removal в turndown использует module-levelisVoid(опцией не переопределить) → бездетные atom-<div>(pageBreak/transclusionReference) попадали в blankRule и исчезали → починеноfillEmptyAtomBlocks(zero-width space, как существующийfillEmptyFootnoteRefs); (2) mention/status не пустые, но падали в дефолт-правило, теряяdata-id/data-color. Lossless-представление: каждый узел реэмитится как инертный raw-HTML из ВСЕХ атрибутов (serializeAttrs), inline — с экранированным текстом. Round-trip подтверждён (markdownToHtml пропускает raw-HTML,parseHTMLкаждого узла восстанавливает ноду).persist-6 — пустой live-doc затирал непустой контент
persistence.extension.ts: store-side empty-guard вonStoreDocument(внутри транзакции, после unchanged-short-circuit, до saveHistory/updatePage): еслиisEmptyParagraphDocИpage.contentнепустой И нетcontext.intentionalClear→ warn + skip записи. Зеркалит guard изonLoadDocument. Легитимные очистки не сломаны:intentionalClear===trueпишет; новые/пустые страницы не затронуты (guard требует непустой существующий контент); empty-over-empty отсекается раньше как deep-equal.Тесты
editor-ext vitest 197 pass (turndown.dataloss 8/8, no .fails); server persistence-store 9/9 (no .failing) + intentional-clear escape + empty-over-empty; collaboration 65, import round-trip 59. editor-ext + server tsc чисто.
Часть B (большой тест-бэклог) — НЕ в этом PR, остаётся открытой в #244.
🤖 Generated with Claude Code
Round-1 review F1 (maintainer decision: variant A). The store-side empty-guard's `context?.intentionalClear === true` branch was dead: `intentionalClear` is never set in production (connection context is {user, actor, aiChatId}); it appeared only in the guard and a hand-injected spec, so the guard already blocked empty-over-non-empty unconditionally. - persistence.extension.ts: drop the dead branch; the guard now simply skips empty-over-non-empty, full stop. Reference issue #251 (real intentional-clear UX) in the comment where the branch was. - persistence-store.spec.ts: remove the misleading "persists an intentional clear" escape-hatch test (false coverage — green only because the flag was injected by hand). Real guard tests (empty-over-empty allowed, empty-over-non-empty blocked, etc.) kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>The "does not block an empty store over an already-empty page" test set the stored page.content to TiptapTransformer.fromYdoc(document,'default') — exactly the value tiptapJson is computed from — so isDeepStrictEqual(tiptapJson, page.content) was TRUE and onStoreDocument RETURNED at the unchanged short-circuit before ever reaching the empty-guard. It exercised the old short-circuit, not the new guard's `!isEmptyParagraphDoc(page.content)` branch (the only NEW branch protecting empty existing pages from over-blocking); the condition could be removed and the test would still pass (false coverage). Set stored content to an empty paragraph with `content: []` — empty per isEmptyParagraphDoc but NOT deep-equal to the live doc (which normalizes to a paragraph with `attrs: { indent: 0 }` and no content key). Execution now skips the short-circuit and enters the guard; reorient the assertion to "the write is NOT blocked" (updatePage IS called). Verified the test now FAILS if the `!isEmptyParagraphDoc(page.content)` condition is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Ревью
90a3fa012— переревью ПОЛНЫМИ 8 аспектами (отдельный субагент на каждый: security, stability, regressions, test-coverage, conventions, documentation, simplification, architecture). Вердикт: approved.Все 8 аспектов — LGTM. Подтверждено: XSS/инъекций нет (экранирование атрибутов корректно, импорт через schema/DOMPurify), empty-guard в правильном слое (в транзакции под lock, не глотает легитимное, empty-over-empty доходит до guard и проходит), round-trip lossless и обратно-совместим для документов без новых узлов, оба бага покрыты регрессионными тестами (падали бы на pre-fix), guard-комментарии и ссылка на #251 точны. Открытых находок нет.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.