diff --git a/apps/client/src/features/editor/gitmost/gitmost-recording.test.ts b/apps/client/src/features/editor/gitmost/gitmost-recording.test.ts index 1f5feef8..f357b070 100644 --- a/apps/client/src/features/editor/gitmost/gitmost-recording.test.ts +++ b/apps/client/src/features/editor/gitmost/gitmost-recording.test.ts @@ -9,7 +9,7 @@ import { Italic } from "@tiptap/extension-italic"; import { Link } from "@tiptap/extension-link"; import { gitmostInsertTranscriptIntoEditor } from "./gitmost-recording.ts"; -const ZWSP = "​"; // U+200B, the helper's block-trigger neutralizer +const ZWSP = "​"; // U+200B — asserted ABSENT (the block-escape lives in the serializer now) /** * #377 — the web-side bridge must append the native host's transcript below the @@ -18,8 +18,9 @@ const ZWSP = "​"; // U+200B, the helper's block-trigger neutralizer * regression would be caught), asserting the resulting document rather than * mocking the editor: transcript present -> "Transcript" heading + one paragraph * per non-empty line; content is inserted as LITERAL TEXT (no HTML/markdown - * parsing); col-0 markdown block triggers are neutralized so git-sync keeps them - * paragraphs; absent/empty/non-string -> no-op. + * parsing); col-0 markdown block triggers are stored verbatim (the git-sync + * serializer block-escapes them, so no client-side ZWSP is needed); + * absent/empty/non-string -> no-op. */ describe("gitmostInsertTranscriptIntoEditor", () => { const makeEditor = () => @@ -91,19 +92,22 @@ describe("gitmostInsertTranscriptIntoEditor", () => { editor.destroy(); }); - it("neutralizes col-0 markdown block triggers with a leading ZWSP (git-sync safety)", () => { + it("inserts col-0 markdown block triggers as verbatim paragraph text (no ZWSP workaround)", () => { const editor = makeEditor(); - // Trigger lines (some with a leaked indent) + a normal prefixed line. + // Trigger lines (some with a leaked indent) + a normal prefixed line. The + // git-sync serializer now block-escapes a leading trigger itself, so the + // bridge inserts each line's TEXT byte-exact (only the leaked indent is + // trimmed) — no invisible ZWSP is prepended anymore. const inserted = gitmostInsertTranscriptIntoEditor( editor, [ "- dash", - " > quote", // leading indent must be trimmed then neutralized + " > quote", // leading indent is trimmed, text otherwise verbatim "# hash", "1. one", "> [!info] note", "```js", - "---", // solid thematic break -> horizontalRule (text-losing) if unneutralized + "---", "***", "___", "You: normal line", @@ -116,20 +120,23 @@ describe("gitmostInsertTranscriptIntoEditor", () => { .map((n: any) => n.content?.[0]?.text) .filter((t: any) => typeof t === "string") as string[]; - // Every block-trigger line is prefixed with the invisible ZWSP (indent - // trimmed first); the normal `You:` line is left byte-exact. + // Each trigger line is stored as its own byte-exact text (indent trimmed); + // the git-sync round-trip keeps it a paragraph via the serializer's + // block-escape, so no ZWSP is needed here. expect(texts).toEqual([ - ZWSP + "- dash", - ZWSP + "> quote", - ZWSP + "# hash", - ZWSP + "1. one", - ZWSP + "> [!info] note", - ZWSP + "```js", - ZWSP + "---", - ZWSP + "***", - ZWSP + "___", + "- dash", + "> quote", + "# hash", + "1. one", + "> [!info] note", + "```js", + "---", + "***", + "___", "You: normal line", ]); + // Guard: no invisible ZWSP leaked into any inserted line. + for (const t of texts) expect(t).not.toContain(ZWSP); editor.destroy(); }); diff --git a/apps/client/src/features/editor/gitmost/gitmost-recording.ts b/apps/client/src/features/editor/gitmost/gitmost-recording.ts index 2856ac08..1be0d638 100644 --- a/apps/client/src/features/editor/gitmost/gitmost-recording.ts +++ b/apps/client/src/features/editor/gitmost/gitmost-recording.ts @@ -240,45 +240,22 @@ export async function gitmostUploadFileToEditor( } } -// Zero-width space (U+200B). Prepended to a transcript line that begins with a -// markdown BLOCK trigger: it is invisible in the rendered doc but shifts the -// trigger off column 0, so the git-sync doc->markdown->doc round-trip keeps the -// line a plain paragraph (see GITMOST_MD_BLOCK_TRIGGER_RE). -const GITMOST_ZWSP = "​"; - -// A markdown BLOCK-level construct that, sitting at column 0 of a paragraph -// line, the git-sync markdown serializer (packages/prosemirror-markdown -// markdown-converter.ts, `case "paragraph"`) would re-parse into a NON-paragraph -// block on the doc->markdown->doc cycle. That serializer emits paragraph text -// verbatim with NO block-escape (the pre-existing root cause), so a leading -// `#`/`-`/`*`/`+`/`>`, an ordered-list `N.`/`N)`, a code fence ```/~~~, a table -// `|`, or a `> [!info]` callout opener would silently become a heading / list / -// quote / code block / table / callout. The final alternative matches a WHOLE- -// LINE thematic break — solid `---`/`***`/`___` or spaced `- - -`/`_ _ _` (3+ of -// the same `-`/`*`/`_`) — which round-trips into a `horizontalRule`; because -// that node carries NO text, an un-neutralized separator line would LOSE its -// text entirely (worse than the list/quote case). This matches a TRIMMED line's -// start; the transcript's own `You:` / `Speaker N:` prefix begins with a letter -// and never matches, so prefixed lines are left byte-exact. -const GITMOST_MD_BLOCK_TRIGGER_RE = - /^(?:#{1,6}(?:\s|$)|[-*+](?:\s|$)|>|\d+[.)](?:\s|$)|```|~~~|\||([-*_])(?:\s*\1){2,}\s*$)/; - // Append a transcript block BELOW the recording's audio node in a live editor: // a "Transcript" heading followed by one paragraph per non-empty transcript // line. The transcript is plain text, `\n`-separated, each line already // formatted as `You: ...` / `Speaker N: ...` by the native host — line text is // inserted as a TEXT node (never HTML/markdown), so there is no injection or // mark-parsing surface. Each kept line is trimmed (drops an indent that would -// both leak into the display and, at col 0, form a markdown block trigger) and, -// if it still begins with a col-0 markdown block trigger, gets an invisible -// zero-width space prepended so the git-sync round-trip cannot turn it into a -// list/quote/heading/callout/code/table (defensive boundary against the -// serializer's missing block-escape). This is best-effort and meant to run -// AFTER the audio has already been inserted; the caller must guard against a -// throw so a transcript failure never fails the (already successful) recording. -// Returns true when a block was inserted, false when there was nothing to -// insert (transcript undefined/empty/not-a-string). A non-string value is a -// no-op, not an error. +// leak into the display). A line that begins with a col-0 markdown block +// trigger (`#`/`-`/`>`/`1.`/fence/`---`/…) needs no client-side workaround: the +// git-sync serializer (packages/prosemirror-markdown, `case "paragraph"`) now +// block-escapes such a leading trigger, so the doc->markdown->doc round-trip +// keeps the line a paragraph on its own — the former invisible-ZWSP defense is +// gone. This is best-effort and meant to run AFTER the audio has already been +// inserted; the caller must guard against a throw so a transcript failure never +// fails the (already successful) recording. Returns true when a block was +// inserted, false when there was nothing to insert (transcript +// undefined/empty/not-a-string). A non-string value is a no-op, not an error. export function gitmostInsertTranscriptIntoEditor( editor: Editor, transcript: unknown, @@ -288,13 +265,7 @@ export function gitmostInsertTranscriptIntoEditor( .split("\n") // Trim each line and drop blank (whitespace-only) ones. .map((line) => line.trim()) - .filter((line) => line.length > 0) - // Neutralize a col-0 markdown block trigger with an invisible ZWSP so the - // git-sync round-trip keeps the line a paragraph. Host lines (`You:` / - // `Speaker N:`) never match and stay byte-exact. - .map((line) => - GITMOST_MD_BLOCK_TRIGGER_RE.test(line) ? GITMOST_ZWSP + line : line, - ); + .filter((line) => line.length > 0); if (lines.length === 0) return false; const content = [ diff --git a/packages/prosemirror-markdown/src/lib/markdown-converter.ts b/packages/prosemirror-markdown/src/lib/markdown-converter.ts index 8389d2b0..0ee31d39 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-converter.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-converter.ts @@ -63,6 +63,54 @@ export interface ConvertProseMirrorToMarkdownOptions { * separator is emitted for any other join, so non-list output is unchanged. */ const LIST_MARKER_SEPARATOR = ""; + +/** + * Backslash-escape a leading markdown BLOCK trigger so a serialized paragraph + * line re-parses as a PARAGRAPH, not another block. Without this, a paragraph + * whose text begins at column 0 with an ATX heading `#`, a blockquote/callout + * `>`, a bullet marker `-`/`*`/`+`, an ordered marker `N.`/`N)`, a code fence + * (```` ``` ````/`~~~`), a table `|`, or a thematic break (`---`/`***`/`___`, + * solid or spaced) silently becomes a heading/list/quote/code block/table/rule + * on the next markdown -> ProseMirror import — a known data-loss class (the + * thematic-break case drops the text entirely, since a horizontalRule carries + * none). CommonMark's escape tokenizer decodes the inserted `\` back to the + * literal character on import AND stops the block interpretation, so the line + * round-trips byte-exact as paragraph text. Only the FIRST offending character + * is escaped (the minimum needed to break block recognition); a line that does + * NOT open a block — emphasis `**x**`, an inline code span, ordinary prose — is + * returned verbatim, so there is no backslash churn for the common case. + * + * Applied ONLY to paragraph text: headings/lists/blockquotes legitimately open + * with these markers and render them from their own cases. This is the single, + * canonical fix for the class the client bridge worked around with a ZWSP + * (`gitmost-recording.ts`) and the generative suite self-censored around + * (`text-arbitraries.ts`) — both now removed. + */ +function escapeLeadingBlockTrigger(line: string): string { + // ATX heading: 1..6 `#` then whitespace/EOL. + if (/^#{1,6}(?:\s|$)/.test(line)) return "\\" + line; + // Blockquote / Docmost callout opener (`>` or `> [!info]`). + if (line.startsWith(">")) return "\\" + line; + // Bullet list marker then whitespace/EOL. Emphasis (`*x*`, `**x**`) has no + // space after the leading marker and is intentionally left verbatim. + if (/^[-*+](?:\s|$)/.test(line)) return "\\" + line; + // Ordered list marker `N.` / `N)`: escape the DELIMITER so the digits stay + // literal (`1. x` -> `1\. x`, which imports back as the text `1. x`). + const ordered = line.match(/^(\d+)[.)](?:\s|$)/); + if (ordered) { + const digits = ordered[1].length; + return line.slice(0, digits) + "\\" + line.slice(digits); + } + // Fenced code block: 3+ backticks or tildes. A single/double backtick is an + // inline code span and is left verbatim. + if (/^(?:`{3,}|~{3,})/.test(line)) return "\\" + line; + // Thematic break: a WHOLE line of 3+ identical `-`/`*`/`_`, optionally spaced. + if (/^([-*_])(?:\s*\1){2,}\s*$/.test(line)) return "\\" + line; + // GFM table row opener. + if (line.startsWith("|")) return "\\" + line; + return line; +} + function listMarkerFamily(type: string | undefined): "ul" | "ol" | null { if (type === "bulletList" || type === "taskList") return "ul"; if (type === "orderedList") return "ol"; @@ -412,7 +460,10 @@ export function convertProseMirrorToMarkdown( } case "paragraph": { - const text = renderInlineChildren(nodeContent); + // Escape a leading block trigger so a paragraph whose text opens with + // `#`/`-`/`>`/`1.`/`|`/a fence/`---` round-trips as a paragraph instead + // of silently re-parsing into another block on the next import. + const text = escapeLeadingBlockTrigger(renderInlineChildren(nodeContent)); const align = node.attrs?.textAlign; // Non-default alignment round-trips as an ATTACHED HTML comment at the // END of the block line (#293 canon #9): diff --git a/packages/prosemirror-markdown/test/generative/text-arbitraries.ts b/packages/prosemirror-markdown/test/generative/text-arbitraries.ts index 76353b76..219b95b2 100644 --- a/packages/prosemirror-markdown/test/generative/text-arbitraries.ts +++ b/packages/prosemirror-markdown/test/generative/text-arbitraries.ts @@ -212,14 +212,42 @@ export function normalizeInline(nodes: any[]): any[] { return out; } +/** + * #493 commit 1: a plain-text run whose text DELIBERATELY OPENS with a markdown + * BLOCK trigger — ATX heading `#`, bullet `-`/`*`/`+`, blockquote `>`, ordered + * `N.`/`N)`, or a table `|` — followed by safe text. Pre-#493 the corpus + * self-censored these away (safeTextArb's leading-word guarantee); the paragraph + * serializer now BLOCK-ESCAPES a leading trigger, so the generative round-trip + * itself proves the data-loss class is closed rather than avoiding it. + * + * DELIBERATELY excludes the code-fence (backtick) trigger — the backtick is a + * code-span delimiter that re-pairs globally (see specialCharArb's note), an + * instability UNRELATED to block-escape — and the whole-line thematic break + * (`---`), which only triggers when the line is ONLY dashes; both are covered by + * the deterministic pin (gitmost-transcript-neutralization.test.ts). Each still + * ENDS in a word (safeTextArb) so adjacent-run concatenation stays safe. + */ +export const blockTriggerLeadRunArb: fc.Arbitrary = fc + .tuple( + fc.constantFrom('# ', '## ', '- ', '* ', '+ ', '> ', '1. ', '1) ', '| '), + safeTextArb, + ) + .map(([trigger, rest]) => ({ type: 'text', text: trigger + rest })); + /** * Inline content for a paragraph: at least one marked text run, optionally with - * inline atoms (math/mention) and hard breaks interspersed. Always starts with a - * text run so the paragraph never opens with a block trigger. (Ported.) + * inline atoms (math/mention) and hard breaks interspersed. The FIRST run is + * usually an ordinary marked run, but sometimes a block-trigger-leading run + * (blockTriggerLeadRunArb) so the paragraph OPENS with a markdown block trigger + * — exercising the serializer's leading block-escape end-to-end. (Ported, with + * the #493 leading-trigger dimension added.) */ export const inlineContentArb: fc.Arbitrary = fc .tuple( - markedTextRunArb, + fc.oneof( + { weight: 5, arbitrary: markedTextRunArb }, + { weight: 1, arbitrary: blockTriggerLeadRunArb }, + ), fc.array( fc.oneof( { weight: 5, arbitrary: markedTextRunArb }, diff --git a/packages/prosemirror-markdown/test/gitmost-transcript-neutralization.test.ts b/packages/prosemirror-markdown/test/gitmost-transcript-neutralization.test.ts index 6fb35f1f..002afe90 100644 --- a/packages/prosemirror-markdown/test/gitmost-transcript-neutralization.test.ts +++ b/packages/prosemirror-markdown/test/gitmost-transcript-neutralization.test.ts @@ -5,32 +5,21 @@ import { convertProseMirrorToMarkdown } from "../src/lib/markdown-converter.js"; import { markdownToProseMirror } from "../src/lib/markdown-to-prosemirror.js"; /** - * gitmost #377 (round-1 review, finding #1) — proof, against the REAL - * converter, that the transcript-insert boundary defense survives git-sync. + * #493 commit 1 — the paragraph serializer's leading-block-escape closes the + * data-loss class where a paragraph whose text opens at column 0 with a markdown + * block trigger (`#`/`-`/`*`/`+`/`>`, an ordered `N.`/`N)`, a code fence, a + * table `|`, a callout opener, or a thematic break) silently re-parsed into a + * heading / list / quote / code block / table / horizontalRule on the git-sync + * doc -> markdown -> doc cycle. The thematic-break case was the worst: a + * horizontalRule carries NO text, so the line's text was lost entirely. * - * The web bridge (apps/client .../gitmost/gitmost-recording.ts, - * `gitmostInsertTranscriptIntoEditor`) appends each transcript line as a - * PARAGRAPH text node. The paragraph serializer here (`case "paragraph"`) emits - * that text VERBATIM with no block-escape, so a line whose text begins with a - * col-0 markdown block trigger would, on the doc -> markdown -> doc git-sync - * cycle, silently re-parse into a heading / list / quote / callout / code block. - * That missing block-escape is the pre-existing root cause; the bridge's - * boundary defense prepends an invisible zero-width space (U+200B) to a line - * that begins with such a trigger, shifting it off column 0. - * - * This test keeps a COPY of the bridge's trigger regex (the bridge is in a - * different package and can't be imported here) and asserts: - * 1. bare trigger lines DO corrupt (documents the root cause), and - * 2. the ZWSP-neutralized form round-trips as a single PARAGRAPH with the - * text byte-preserved. + * This is the deterministic PIN, one assertion per trigger, exercised through + * the REAL converter round-trip (not a mock): each bare trigger line now + * round-trips as a SINGLE paragraph with its text byte-preserved — proving the + * class is closed WITHOUT the former client-side ZWSP workaround (removed) or + * the generative suite's leading-word self-censorship (removed). */ -const ZWSP = "​"; // U+200B - -// MUST stay in sync with GITMOST_MD_BLOCK_TRIGGER_RE in the client bridge. -const MD_BLOCK_TRIGGER_RE = - /^(?:#{1,6}(?:\s|$)|[-*+](?:\s|$)|>|\d+[.)](?:\s|$)|```|~~~|\||([-*_])(?:\s*\1){2,}\s*$)/; - const doc = (...nodes: any[]) => ({ type: "doc", content: nodes }); const para = (t: string) => ({ type: "paragraph", @@ -43,78 +32,74 @@ const roundtrip = async (text: string) => { return back.content as any[]; }; -describe("gitmost transcript neutralization (git-sync round-trip)", () => { - // Lines that, at column 0, the serializer's missing block-escape would let - // git-sync re-parse into a non-paragraph block. +describe("paragraph block-escape (git-sync round-trip)", () => { + // Every line here, at column 0, WOULD (pre-fix) re-parse into a non-paragraph + // block. Each is now block-escaped by the serializer and round-trips clean. const triggerLines = [ "- dash", "* star", "+ plus", "> quote", "# hash", + "## two hash", + "###### six hash", "1. one", "1) one", "> [!info] note", "```js", "~~~", - // Solid + spaced thematic breaks — these re-parse into a `horizontalRule`, - // which carries NO text, so a bare separator line LOSES its text entirely - // (round-2 finding). `_` also only forms a block via this construct. + "| a | b |", + // Solid + spaced thematic breaks — the text-LOSING case pre-fix. "---", "***", "___", - "- - -", // spaced dash break (solid form is caught by [-*+]\s too, but this is the break) + "- - -", "_ _ _", ]; - it("BARE trigger lines corrupt into non-paragraph blocks (root cause)", async () => { + it("every bare trigger line round-trips as a single paragraph, text byte-preserved", async () => { for (const line of triggerLines) { const blocks = await roundtrip(line); - // At least one produced block is NOT a paragraph — i.e. corruption. - const allParagraphs = blocks.every((b) => b.type === "paragraph"); - expect( - allParagraphs, - `expected "${line}" to corrupt when inserted bare`, - ).toBe(false); - } - }); - - it("BARE solid thematic breaks corrupt into a text-LOSING horizontalRule", async () => { - // The severe case: no text node survives. Documents why neutralization - // matters more here than for list/quote (where the text survived). - for (const line of ["---", "***", "___"]) { - const blocks = await roundtrip(line); - expect(blocks.map((b) => b.type)).toContain("horizontalRule"); - // No block carries the original text anywhere. - const flat = JSON.stringify(blocks); - expect(flat).not.toContain(line); - } - }); - - it("ZWSP-neutralized trigger lines round-trip as a single paragraph, text preserved", async () => { - for (const line of triggerLines) { - // The regex must actually classify each as a trigger. - expect(MD_BLOCK_TRIGGER_RE.test(line), `regex missed "${line}"`).toBe( - true, + expect(blocks, `"${line}" should be one block`).toHaveLength(1); + expect(blocks[0].type, `"${line}" should stay a paragraph`).toBe( + "paragraph", ); - const neutralized = ZWSP + line; - const blocks = await roundtrip(neutralized); - - expect(blocks).toHaveLength(1); - expect(blocks[0].type).toBe("paragraph"); - // Text is byte-preserved (ZWSP + original line), so the display is the - // original line with only an invisible leading character. - expect(blocks[0].content[0].text).toBe(neutralized); + expect( + blocks[0].content?.[0]?.text, + `"${line}" text should survive byte-exact`, + ).toBe(line); } }); - it("normal host-prefixed lines never match the trigger regex and round-trip byte-exact", async () => { + it("emphasis / inline-code paragraphs are NOT escaped (no backslash churn)", async () => { + // These open with `*`/`` ` `` but are NOT block triggers; the serialized + // markdown must not gain a stray leading backslash, and they round-trip. + for (const [text, mark] of [ + ["bold", "bold"], + ["italic", "italic"], + ["code", "code"], + ] as const) { + const node = doc({ + type: "paragraph", + content: [{ type: "text", text, marks: [{ type: mark }] }], + }); + const md = convertProseMirrorToMarkdown(node); + expect(md.startsWith("\\"), `${mark} must not be block-escaped`).toBe( + false, + ); + const back = await markdownToProseMirror(md); + expect(back.content[0].type).toBe("paragraph"); + expect(back.content[0].content[0].text).toBe(text); + expect(back.content[0].content[0].marks?.[0]?.type).toBe(mark); + } + }); + + it("normal host-prefixed lines round-trip byte-exact (unaffected)", async () => { for (const line of [ "You: hello there", "Speaker 1: - and then a dash mid-line", "Speaker 2: 1. not a list", ]) { - expect(MD_BLOCK_TRIGGER_RE.test(line)).toBe(false); const blocks = await roundtrip(line); expect(blocks).toHaveLength(1); expect(blocks[0].type).toBe("paragraph"); diff --git a/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts b/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts index c3bb1e2b..9008c34b 100644 --- a/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts +++ b/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts @@ -430,7 +430,7 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => { }); }); -describe('converter gap coverage — documented round-trip data loss (specs 12–14)', () => { +describe('converter gap coverage — formerly-lossy round-trips, now closed (specs 12–14)', () => { // 12. A 3-backtick fence inside a codeBlock body is now lengthened: the outer // fence widens to (longest inner run + 1) backticks per CommonMark, so the // inner ``` is treated as content and the block survives as ONE node. @@ -460,25 +460,24 @@ describe('converter gap coverage — documented round-trip data loss (specs 12 expect(docsCanonicallyEqual(d, doc2)).toBe(false); }); - // 13. A leading ordered-list marker in paragraph text is NOT escaped, so a - // plain paragraph silently becomes an orderedList on re-import. - it('a paragraph starting with "1. " is promoted to an orderedList on re-import', async () => { + // 13. #493 commit 1: a leading ordered-list marker in paragraph text is now + // BLOCK-ESCAPED, so the paragraph round-trips as a paragraph instead of + // silently becoming an orderedList (was documented data loss, now closed). + it('a paragraph starting with "1. " is block-escaped and stays a paragraph', async () => { const d = doc({ type: 'paragraph', content: [{ type: 'text', text: '1. not a list' }], }); const md1 = convertProseMirrorToMarkdown(d); - expect(md1).toBe('1. not a list'); // no backslash escape + expect(md1).toBe('1\\. not a list'); // the ordered-list delimiter is escaped const doc2 = await markdownToProseMirror(md1); - expect(doc2.content?.[0]?.type).toBe('orderedList'); - const li = doc2.content[0].content?.[0]; - expect(li?.type).toBe('listItem'); - expect(li.content?.[0]?.content?.[0]).toMatchObject({ + expect(doc2.content?.[0]?.type).toBe('paragraph'); + expect(doc2.content[0].content?.[0]).toMatchObject({ type: 'text', - text: 'not a list', // the "1. " was consumed as a list marker + text: '1. not a list', // the escape decodes back to the literal text }); - expect(docsCanonicallyEqual(d, doc2)).toBe(false); + expect(docsCanonicallyEqual(d, doc2)).toBe(true); }); // 14. #293 canon #4: the image title now round-trips via the attached