diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts index e4e448be..4ef94bbf 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-content-integrity.test.ts @@ -135,4 +135,36 @@ describe('generative content-integrity — atoms survive load↔serialize across expect(out.includes(atom), `atom "${atom}" lost (i=${i}):\n${doc}\n-->\n${out}`).toBe(true); } }); + + // A raw `|` inside a table cell is the one context the single-atom oracle above cannot generate: marked + // splits the row on it before inline-tokenizing, so an unprotected pipe drops the trailing cells. Build + // tables whose cells carry pipe-bearing atoms inside code spans / Go actions, plus a sentinel trailing + // cell; assert the sentinel and the atom's words survive and the table converges. + it('pipe-bearing atoms inside table cells keep their row intact and converge', () => { + const rng = mulberry32(0x7ab1e); + const pipeAtoms = ['x | y', '{{.Host | lower}}', 'a || b', 'grep foo | wc -l', 'no-pipe-here']; + + for (let i = 0; i < 120; i++) { + const rowCount = 1 + Math.floor(rng() * 3); + const rows: string[] = []; + const sentinels: string[] = []; + + for (let row = 0; row < rowCount; row++) { + const atom = pipeAtoms[Math.floor(rng() * pipeAtoms.length)] as string; + const sentinel = `${WORDS[Math.floor(rng() * WORDS.length)]}${row}`; + + sentinels.push(sentinel); + rows.push(`| \`${atom}\` | ${sentinel} |`); + } + + const doc = `| code | note |\n| --- | --- |\n${rows.join('\n')}`; + const out = roundTrip(doc); + + expect(roundTrip(out), `did not converge (i=${i}):\n${doc}\n-->\n${out}`).toBe(out); + + for (const sentinel of sentinels) { + expect(out.includes(sentinel), `cell "${sentinel}" dropped (i=${i}):\n${doc}\n-->\n${out}`).toBe(true); + } + } + }); }); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts index 7bfbd058..0f8e903e 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-marked.ts @@ -2,9 +2,11 @@ import type { JSONContent, MarkdownRendererHelpers } from '@tiptap/core'; import { Extension } from '@tiptap/core'; import { renderTableToMarkdown, Table } from '@tiptap/extension-table'; -import { Markdown } from '@tiptap/markdown'; +import { Markdown, MarkdownManager } from '@tiptap/markdown'; import { Marked } from 'marked'; +import { escapeTablePipes } from './markdown-editor-table-pipes'; + // @tiptap/markdown parses with `marked`, which — unlike markdown-it's `html: false` — always tries to // interpret `<...>` as HTML. Our content uses literal XML-ish tags (``, ``) // that must survive verbatim; marked silently swallows the ones whose names match real HTML elements @@ -66,12 +68,36 @@ const TunedMarkdownText = Extension.create({ priority: 50, }); +// The load-side counterpart to TunedTable's save-side pipe escape: protect a `|` inside a code span / Go +// action in a table cell BEFORE marked's table tokenizer splits the row (it splits raw `|` before inline +// tokenization, dropping trailing cells). This has to patch MarkdownManager.parse on the PROTOTYPE, not the +// instance: the Markdown extension parses the INITIAL `content` inside its own onBeforeCreate (before any +// lower-priority extension can touch the manager), so an instance patch would miss the initial load. The +// manager also lexes via `new Lexer()`, bypassing Marked.parse — so a marked-level hooks.preprocess never +// runs. Idempotent + guarded; the escape is a no-op fast-path for content without a table. +let isManagerParsePatched = false; + +const patchManagerParse = () => { + if (isManagerParsePatched) { + return; + } + + isManagerParsePatched = true; + + const proto = MarkdownManager.prototype as unknown as { parse: (markdown: string) => unknown }; + const parse = proto.parse; + + proto.parse = function tunedParse(markdown: string) { + return parse.call(this, escapeTablePipes(markdown)); + }; +}; + // @tiptap/extension-table's renderTableToMarkdown is alignment-aware but never escapes pipes, so a literal // `|` a cell emits (even from inside inline code) would re-parse as a column delimiter on the next SAVE and // drop cells (tiptap PR #7884). renderTableToMarkdown emits cell content only via h.renderChildren, so wrapping // that one call to escape pipes fixes the save side, on the official alignment-aware renderer. -// This does NOT (and cannot) help the LOAD side: a raw `|` inside inline code arriving in external markdown is -// split by marked's GFM table tokenizer BEFORE the inline-code tokenizer runs — a marked limitation. +// The LOAD side of the same class (a raw `|` inside inline code arriving in external markdown) is covered by +// patchManagerParse above. export const TunedTable = Table.extend({ renderMarkdown(node: JSONContent, helpers: MarkdownRendererHelpers) { const pipeEscaping: MarkdownRendererHelpers = { @@ -83,7 +109,13 @@ export const TunedTable = Table.extend({ }, }); -export const createMarkdownLayer = () => [ - Markdown.configure({ marked: createTunedMarked() as unknown as typeof import('marked').marked }), - TunedMarkdownText, -]; +export const createMarkdownLayer = () => { + // Installed here (before `new Editor` consumes these extensions) so the prototype patch is in place + // when the Markdown extension parses the initial content during construction. + patchManagerParse(); + + return [ + Markdown.configure({ marked: createTunedMarked() as unknown as typeof import('marked').marked }), + TunedMarkdownText, + ]; +}; diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts new file mode 100644 index 00000000..833ed71f --- /dev/null +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from 'vitest'; + +import { escapeTablePipes } from './markdown-editor-table-pipes'; +import { roundTrip, setupEditorJsdom } from './markdown-editor-test-setup'; + +setupEditorJsdom(); + +describe('escapeTablePipes — pure pre-lex pipe protection', () => { + it('escapes a pipe inside a code span in a body row', () => { + expect(escapeTablePipes('| Op | Meaning |\n| --- | --- |\n| `x | y` | z |')).toBe( + '| Op | Meaning |\n| --- | --- |\n| `x \\| y` | z |', + ); + }); + + it('escapes a pipe inside a Go-template action in a body row', () => { + expect(escapeTablePipes('| Var | Out |\n| --- | --- |\n| {{.X | upper}} | ok |')).toBe( + '| Var | Out |\n| --- | --- |\n| {{.X \\| upper}} | ok |', + ); + }); + + it('leaves a plain-text pipe (real column delimiter) untouched', () => { + const table = '| a | b |\n| --- | --- |\n| 1 | 2 |'; + + expect(escapeTablePipes(table)).toBe(table); + }); + + it('does not double-escape an already-escaped pipe', () => { + const table = '| a | b |\n| --- | --- |\n| `x \\| y` | z |'; + + expect(escapeTablePipes(table)).toBe(table); + }); + + it('handles a multi-backtick code span', () => { + expect(escapeTablePipes('| a | b |\n| --- | --- |\n| ``x | y`` | z |')).toBe( + '| a | b |\n| --- | --- |\n| ``x \\| y`` | z |', + ); + }); + + it('leaves an unclosed backtick run alone (not a code span)', () => { + const table = '| a | b |\n| --- | --- |\n| `x | y |'; + + expect(escapeTablePipes(table)).toBe(table); + }); + + it('never touches a fenced code block that happens to hold a table', () => { + const doc = '```\n| a | b |\n| --- | --- |\n| `x | y` | z |\n```'; + + expect(escapeTablePipes(doc)).toBe(doc); + }); + + it('ignores a non-table line whose text contains pipes in code', () => { + const prose = 'run `a | b` in the shell'; + + expect(escapeTablePipes(prose)).toBe(prose); + }); + + it('returns the input unchanged when there is no pipe at all', () => { + const doc = '# heading\n\nsome `code` here'; + + expect(escapeTablePipes(doc)).toBe(doc); + }); +}); + +describe('table cell with a piped code span — content survives load and converges (H1)', () => { + it('keeps the trailing cell and preserves the code content', () => { + const out = roundTrip('| Op | Meaning |\n| --- | --- |\n| `x | y` | z |'); + + expect(out).toContain('z'); + expect(out).toContain('`x \\| y`'); + expect(roundTrip(out)).toBe(out); + }); + + it('keeps a Go-template action with a pipe inside a cell', () => { + const out = roundTrip('| Var | Out |\n| --- | --- |\n| {{.X | upper}} | done |'); + + expect(out).toContain('done'); + expect(out).toContain('{{.X \\| upper}}'); + expect(roundTrip(out)).toBe(out); + }); +}); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts new file mode 100644 index 00000000..f0439045 --- /dev/null +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts @@ -0,0 +1,153 @@ +// marked's GFM table tokenizer splits every row on raw `|` BEFORE inline tokenization, so a pipe inside a +// code span (`` `x | y` ``) or a Go-template action (`{{.X | upper}}`) in a body cell creates a phantom +// column and the trailing cells are silently DROPPED on load. The splitter does honor `\|` — and unescapes +// it in the cell text — so pre-escaping those pipes before marked lexes protects the load side the same way +// TunedTable's renderChildren escape protects the save side. +// +// Scope is deliberately conservative — only rows that marked itself would treat as a table: +// • the header/delimiter pair must already parse as a table (matching cell counts, delimiter has |/:) — +// escaping pipes in a NON-table line would surface literal `\|` (and could even turn a setext heading +// into a table by changing the header's cell count); +// • only BODY rows starting with `|` are transformed (header cell counts are structural; a body row is +// truncated/padded to the header width, so protecting its pipes only ever preserves more content); +// • fenced code blocks are never touched. + +const FENCE_LINE = /^ {0,3}(```|~~~)/; +const TABLE_DELIMITER_LINE = /^ {0,3}\|? *:?-+:? *(?:\| *:?-+:? *)*\|? *$/; +const BODY_ROW_LINE = /^ {0,3}\|/; +const TEMPLATE_ACTION = /\{\{[^{}]*\}\}/g; + +const isEscapedAt = (text: string, offset: number): boolean => { + let isEscaped = false; + let index = offset; + + while (--index >= 0 && text[index] === '\\') { + isEscaped = !isEscaped; + } + + return isEscaped; +}; + +const escapeUnescapedPipes = (text: string): string => + text.replace(/\|/g, (pipe, offset: number, source: string) => (isEscapedAt(source, offset) ? pipe : '\\|')); + +// Mirrors marked's splitCells: split on unescaped pipes, drop a blank leading/trailing cell. +const countCells = (row: string): number => { + const cells = row + .replace(/\|/g, (pipe, offset: number, source: string) => (isEscapedAt(source, offset) ? pipe : ' |')) + .split(/ \|/); + + if (cells.length > 0 && !cells[0]!.trim()) { + cells.shift(); + } + + if (cells.length > 0 && !cells.at(-1)!.trim()) { + cells.pop(); + } + + return cells.length; +}; + +const findCodeSpanCloser = (row: string, from: number, runLength: number): number => { + for (let index = from; index < row.length; index++) { + if (row[index] !== '`') { + continue; + } + + let runEnd = index; + + while (runEnd < row.length && row[runEnd] === '`') { + runEnd++; + } + + if (runEnd - index === runLength) { + return index; + } + + index = runEnd - 1; + } + + return -1; +}; + +const escapeRowPipes = (row: string): string => { + let result = ''; + let index = 0; + + while (index < row.length) { + const char = row[index]!; + + if (char !== '`') { + result += char; + index++; + continue; + } + + let runEnd = index; + + while (runEnd < row.length && row[runEnd] === '`') { + runEnd++; + } + + const run = row.slice(index, runEnd); + const closerStart = findCodeSpanCloser(row, runEnd, run.length); + + // An unclosed backtick run is literal content, not a code span. + if (closerStart < 0) { + result += run; + index = runEnd; + continue; + } + + result += run + escapeUnescapedPipes(row.slice(runEnd, closerStart)) + run; + index = closerStart + run.length; + } + + return result.replace(TEMPLATE_ACTION, (action) => escapeUnescapedPipes(action)); +}; + +export const escapeTablePipes = (markdown: string): string => { + if (!markdown.includes('|')) { + return markdown; + } + + const lines = markdown.split('\n'); + let openFence: null | string = null; + let isChanged = false; + + for (let index = 0; index < lines.length; index++) { + const line = lines[index]!; + const fence = FENCE_LINE.exec(line); + + if (fence) { + openFence = openFence === fence[1] ? null : (openFence ?? fence[1]!); + continue; + } + + if (openFence !== null || !line.includes('|')) { + continue; + } + + const delimiter = lines[index + 1]; + + if ( + delimiter === undefined || + !TABLE_DELIMITER_LINE.test(delimiter) || + !/[|:]/.test(delimiter) || + countCells(line) !== countCells(delimiter) + ) { + continue; + } + + for (let row = index + 2; row < lines.length && BODY_ROW_LINE.test(lines[row]!); row++) { + const escaped = escapeRowPipes(lines[row]!); + + if (escaped !== lines[row]) { + lines[row] = escaped; + isChanged = true; + } + } + } + + return isChanged ? lines.join('\n') : markdown; +};