fix: linear-time hostile markdown and handled nesting refusals (fixes #1407) - #1415
Merged
Merged
Conversation
… time The weekly pathological soak (#1407) was killed at its 300s wall ceiling on link-closers-with-openers. It was not a regression: the same input costs the same at 0ce4d2e, when the suite was added. Earlier soak runs never reached this class, because nested-strong-emph overflowed the stack first (#1374) and, before the matrix split, the fuzz step failed ahead of it. Measured at soak scale 8, the class alone took 60s to parse and 276s to serialize. The CPU profile names two backward scans that micromark runs at EVERY ] in a paragraph: label-end walks back to the nearest unbalanced label start, and gfm-footnote's potential call walks back to the nearest image or link. With no opener both reach the paragraph start and fail, so a ] x n paragraph is O(n^2). Growth measured 4.4x parse per 2x input. The same shape repeats in two more micromark constructs. codeText scans to the paragraph end for a closing run of the same length, so runs of distinct lengths cost O(openers x paragraph); backtick-runs had not finished after twelve minutes. attention's resolver walks every closer back to the start when no opener exists; emph-closers-without-openers took 27s to parse. The fix is micromark constructs that act only on a PROOF that the stock construct would fail, and defer to it otherwise: - at ], consume the bracket as data when no unbalanced label start exists, found from an incremental index. The footnote call cannot succeed at such a ]: its label would contain the unescaped ] that balanced the image, which no footnote definition label can hold. Splices by a stock success are caught by watching the event at the opener index; - at a backtick run, remember per run length where the last run is (cmark's fix for the same class) and consume a run that no later run can close; - for * and _, lower _close on closers that cannot open and have no same-marker opener anywhere before them, for the duration of the stock resolver. Resolving only consumes openers, so their walk can match nothing. inlineFastPaths.test.ts diffs the mdast, positions included, against a parser without the plugin over every vendored spec corpus (947 examples), hand-picked boundary inputs and 3000 seeded fuzz documents. It caught a real error while this was built: a wrapper added in front of gfm's email autolink turned (_A_@_.A into emphasis, so the attention wrapper is appended and refuses to run anywhere but directly behind the disabled stock construct. Each fast path's decision was also mutated to confirm the test fails on it. The plugin is registered in every parse mode, including the details-body chain that is built from leaf imports.
…arkdown With the inline fast paths in, two classes of the pathological soak (#1407) were still super-linear, and both costs sit where no extension API reaches. Both packages are at their latest releases. micromark 4.0.2, text initializer resolveAllText. It merges adjacent data tokens with one events.splice per run, and each splice moves every later event. That is O(runs x events), and it turns sharply worse once a paragraph's event list outgrows V8's regular heap objects: plain escaped prose, the shape of the serializer's own conservative output, parsed in 546ms at 72 KB and 5.5s at 96 KB. It is also what kept unclosed-inline-links quadratic, through the cosmetic pass re-parsing that output. The patch merges in one compacting pass that keeps the same events, sets the same end point on the merged token, and returns the same array. Both the lib and dev builds are patched. mdast-util-to-markdown 2.1.2, safe(). positions.includes(position) runs once per unsafe-pattern match, so escaping is O(escapes^2); a text node of 116k backticks took 15s to serialize. infos gains a key exactly when positions gains the position, so an own-key check is the same test in O(1). Verified against the unpatched copies still in the store: micromark event streams, positions included, were identical over all 947 spec corpus examples and 70000 seeded fuzz documents across the lib and dev builds, and safe() returned identical strings for 100000 fuzzed calls. A version bump of either package now fails install until the patch is re-made. That is deliberate: a patch that silently stopped applying would bring the quadratic back, and pathologicalScaling.test.ts would then fail. The lockfile diff is the patch records and hashes only.
The serializer's cosmetic pass decides, for every defensive escape, whether only whitespace precedes it on its line. It did that with a lastIndexOf back to the line start plus a slice per escape: O(n^2) on a long line of escapes. Every hostile inline class in the pathological soak (#1407) serializes to exactly such a line, and on backtick-runs this loop was the largest single cost in the serializer profile. One forward pass now tracks the current line start and the last non-whitespace index as it moves between matches. \s is the set trimStart removes, so the decision is the same one as before. Adding the pass took serializerCosmetics.ts over the 300-line limit, so the code-range helpers move to serializerCodeRanges.ts. They are the natural seam: the hard-break pass in serializer.ts uses them too, and nothing in them depends on the cosmetic pass.
…R tier The only place a super-linear parse or serialize ever showed up was the weekly soak, and it showed up as a hang (#1407). This brings the property into check:all. It asserts a growth EXPONENT, never a duration: wall-clock bounds flake under contention, which is why performance.test.ts is opt-in. Contention slows a small and a large run alike, but it cannot turn cost proportional to n into n^2, which for an 8x input is the gap between ~8x and ~64x the CPU time. Process CPU time, interleaved runs keeping the minimum, and a warm-up keep the measurement honest under load. Eight classes: link closers with no opener, after an image, after a footnote-like image with footnotes defined, and between real links; unclosed inline links; emphasis closers without openers; backtick runs; and escaped prose past V8's large-object threshold (parse only, for cost). Measured at a load average above 150, every class is 1.04 to 1.18 with the fixes. Each was RED first: 1.82 to 2.35 against the unfixed pipeline, 1.41 for backtick-runs, 1.61 for the footnote-like image against the first version of the ] fast path, and 1.55 for the between-links case with the fast path's re-index watch removed. backtick-runs carries a tighter bound of 1.25, because its broken cost is n^3 in the run count, which is size^1.5 rather than size^2.
…ut sweep The OSS-Fuzz leg of the corpora soak (#1407) failed on a corpus file nested 16382 levels deep, reproduced locally with the same error. The pipeline refuses such a document on purpose: past MAX_NESTING_DEPTH the parser's own dependencies overflow the stack, and 8c6bc00 made that a named NestingTooDeepError instead of a RangeError (#1374). The soak treated the designed refusal as a crash. The pathological runner already classified it, by hand. Both sweeps now go through one helper, parseArbitraryMarkdown, which returns a refusal as data and rethrows every other failure. The classification is by type along the cause chain, never by message: nestingRefusal() reads the depth and limit off the error, and an error that merely says it is a nesting refusal is not one. Only a sweep's INPUT may be refused. The soak's re-parses read the pipeline's own output, where a refusal would be a real defect, so they still throw. The soak reports refused inputs loudly, measures the oscillation fraction over the inputs it actually round-tripped, and requires more than 100 of them, so a refusal cannot eat the sample and pass. The same sweep had a second, latent failure the hang was hiding. At the soak's scale 8, deep-blockquotes (4000 levels) and deep-lists (1600) are refused, but the parent test only counted parsed classes as finished, so it would have failed on them next; reproduced at PATHOLOGICAL_SCALE=2.1. Each case now declares its container depth from its generator, and the parent requires exactly those above the limit to be refused and every other class to parse. A refusal nobody expected fails instead of passing as finished. The scale is read in one place, so child and parent cannot disagree. The other property and fuzz tests that parse markdown build inputs from fixed pools that cannot nest anywhere near the limit, so they need no classification.
…a message Following the nesting refusal from #1407 into the app: when a user opens a file nested past MAX_NESTING_DEPTH, TiptapEditor caught the parse failure, logged it to the console with tiptapError, and carried on with an EMPTY document. That editor was live. Its first keystroke flushed the empty document to the store, so typing would have saved over the file the user had just opened, and nothing told them why the editor was blank. An external content change that failed to parse had the same hazard: the editor kept the old content, and the next edit would overwrite the new text. Both failure paths now go to reportUnparseableDocument, which puts the tab in Source mode through the existing per-tab forced-Source marker and shows an error toast. Source mode shows the text as it is and needs no parse, so it is the one place the document is both visible and safely editable. A nesting refusal says how deep the document is and what the limit is; any other parse failure gets a general message, because the blank editor is the same hazard whatever the cause. The marker now records why a tab is forced: large-file, as before, or unparseable. The status line says "Opened in Source mode (cannot be displayed in WYSIWYG)." for a refusal instead of claiming the file is large, and keeps Switch to WYSIWYG, which is how the user asks again after reducing the nesting. A split view parses one document in several editors, so the report happens once per tab, and a tab already forced for its size keeps that reason. Existing editor tests that asserted the old dev-only log now assert the report. One of them left its deferred parse pending with a one-shot throwing mock, which leaked into whichever later test ran timers next; it now runs its timers itself. The new strings are translated into all ten locales, and the Large Files guide documents the behavior in every language.
…vent walk The attention prefilter from the first fast-path commit only helped closers with no same-marker opener anywhere before them. deep-inline-nesting, the tenth pathological class, has the same futile walk in a form a static check cannot see: earlier matches CONSUME the only candidate openers, so two of every three closers still walked back to the paragraph start. Measured at soak scale 8: 117s to parse. The resolver is now micromark-core-commonmark 2.0.3's resolveAllAttention, ported with one change. Instead of walking every event, a closer scans a per-marker list of the opener sequences still present before it, nearest first, applying the stock test to each; the match and everything done with it (tokens, points, the inner span resolved through insideSpan, the splice, where the loop resumes) is the stock code. The list is exact: an opener joins it when the loop passes its exit event, and a match truncates EVERY marker's list past the opener, because the splice rewrites everything after it and a stale entry could otherwise alias a later sequence at a shifted index. A mutation that scanned the list in the wrong order failed the equivalence test. deep-inline-nesting now parses in 0.8s, and the whole scale-8 pathological run takes 10.8s where link-closers-with-openers alone used to be killed at 300s. Also, from the first audit round: the fast paths' "unknown constructs defer" guard read only the construct list for the character and missed text.null, the constructs micromark tries at every code. A catch-all construct from another extension could then have lost a ] or a backtick to a fast path. The guard reads both lists now, and a test registers such a construct and diffs the trees; it failed before the change.
First audit round on the Source-mode fallback (#1407) reproduced a data loss it did not prevent: with a keystroke's flush pending, an external change the parser refuses leaves the editor holding the OLD document, the tab switches to Source mode, and the unmount flush writes that old document over the new one. The store ended up "stale edit". Pending debounce flushes and Save's flush take the same route. flushToStore now writes nothing for a tab whose forced-Source reason is unparseable. The marker is set synchronously in the failure path, before the tab can re-render, so every later flush from the refused editor sees it. A dropped keystroke was typed into content the store no longer holds; the external change is the text to keep. A tab forced only for its size still writes, and writing resumes once the reason is cleared. Two related corrections from the same round: - A hidden keep-alive editor no longer reports a refusal. It shows nothing and takes no edits, and reporting from it could put a tab the user reads in Source mode into the refused state, where the status line's Switch to WYSIWYG could not switch while global Source mode was on. It re-parses when shown, and reports then. - A refusal on a tab already forced for its size now changes the reason to unparseable, without a second toast: the reason is what stops the writes. The flush tests' helper took an explicit undefined tab as its default "tab-1", so the test that claimed to cover the tab-store fallback never reached it. It passes null for no tab now, and both fallback tests exercise the fallback. The service header also said the old logger was silent in release builds; tiptapError logs through prodError, so it now says the failure reached only the console.
Both sweeps and the editor now read nestingRefusal(), which also returns the depth and the limit, so the boolean wrapper had no caller left outside its own tests (first audit round, #1407). The error class's comment now names nestingRefusal as the interface to use.
…ys linear Three layers now keep it linear (#1407), and each has a way to silently stop doing so: the micromark fast paths, the two pnpm patches, and the scaling test that notices. The note says which is which, what a Dependabot bump of a patched package does (install fails until the patch is re-made, deliberately), and the two classes that are still super-linear, with their measured cost at soak scale, so the next reader does not rediscover them as a regression.
…eText Second audit round (#1407) found a position the backtick fast path changed. In "*<TAB>`x`\n<TAB>`x`" the newline text node ended one column later than stock. The first opener's memo scan ran to the end of the paragraph, but codeText closes on the same line; reading past the line ending reached a chunk the subtokenizer had not written yet, so the container skip it defined for the next line was applied to the lookahead's point instead of the real one. The visible text was unchanged; the positions, which the equivalence test promises, were not. The scan now stops exactly where codeText would: at its first closing run, leaving no memo, or at the end of the paragraph, leaving one. The skipped scans that follow cannot repeat the problem, because a memo exists only after a scan reached the end, and by then every chunk and skip is in place. Four container-continuation cases join the equivalence test, including the audit's input and one a container-heavy fuzz found; three fail with the old scan. Two fuzz runs of 40000 such documents now show no position difference. Also: the extension object and the forced-Source reason type are module-local (knip's baseline counted both as new dead exports), and the equivalence test's tree helper takes a structural processor type, which the test-types gate needed.
…rom reporting Two more findings from the second audit round on the refused-document fallback (#1407), both reproduced by the audit with real stores: - Switch to WYSIWYG still did nothing when the window itself was in Source mode. Refuse document A, turn Source mode on from tab B, return to A, reduce the nesting, click: the marker cleared but global Source mode stayed on. The button now lifts the marker and, when the window is in Source mode, runs the ordinary mode toggle with its checkpoint and per-tab mode record, so other tabs keep their mode when they are next shown. Its old test asserted exactly the no-op; it now asserts the switch. This applies to the large-file offer too, which had the same no-op in the same state. - A hidden keep-alive editor could still report. When its first parse succeeded but the document changed before the deferred work ran, the drift re-sync ran while hidden, and a refused drift reported for a view nobody had opened. That re-sync now waits for visibility, as the content-change effect already did; the hidden-to-visible sync picks it up.
…not a replaced snapshot Third audit round (#1407): an editor mounted hidden with a document the parser refuses defers its first parse. If the user repairs the document and shows the editor before that parse runs, the parse still sees the old snapshot, fails, and reported it, putting the repaired document in Source mode with an empty editor behind it. When the snapshot has been replaced by then, the editor now syncs the latest content instead, and that sync reports only if the latest content is refused too. A hidden editor still reports nothing and re-syncs when shown.
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1407.
Both failing soak suites are green again, and neither was "just raise the ceiling":
link-closers-with-openers. The cause is O(n²) and worse work at every], backtick run and emphasis closer, inside micromark and mdast-util-to-markdown, plus one loop in VMark's serializer. After the fixes, the whole scale-8 run takes 10.8 s (idle machine, 12.4 s CPU). Before, that one class alone took 337 s.Rust
rust-auditis expected to fail on RUSTSEC-2026-0285 (see Not covered).Root causes
1. Pathological soak: super-linear inline tokenization (not a regression)
Not a regression.
link-closers-with-openerscosts the same at0ce4d2e54(when the suite was added), atffb6b7475(just before8c6bc00c4), and ataf945bfb2. Scale 2, interleaved: parse 1345/1466/1507 ms, serialize 12.6/11.4/12.3 s. Earlier soak runs never reached this class:nested-strong-emphoverflowed the stack first (#1374), and before the matrix split the fuzz step failed ahead of it. Reproduced locally ataf945bfb2with the exact CI failure (killed at 300 107 ms, same class, same finished list).Scaling, measured per class (CPU ms, scale 1 → scale 2, meaning 2× input):
At scale 8 (wall, before): link-closers parse 60.7 s plus serialize 276 s. backtick-runs was still running after 12 min when I stopped it. emph-closers took 27 s plus 44 s, and deep-inline-nesting 117 s.
Mechanisms, from CPU profiles:
labelEndand gfm-footnotegfmPotentialFootnoteCalleach walk back over every event at every]. With no opener, both reach the paragraph start. The serializer's cosmetic pass then re-parses its escaped output twice, so serializing costs even more than parsing.codeTextscans to the paragraph end for a same-length closing run. Runs of distinct lengths therefore cost openers × paragraph, which is cubic in the run count.resolveAllAttentionwalks every closer back to the start when it finds no opener, and also when earlier matches already consumed the only candidates (deep-inline-nesting).resolveAllTextmerges adjacent data tokens with onespliceper run: O(runs × events). It gets about 10× slower per step once the event list outgrows V8's regular heap objects. Escaped prose parsed in 546 ms at 72 KB and 5.5 s at 96 KB.safe()runspositions.includesonce per unsafe match, so escaping is O(escapes²). 116 k backticks took 15 s.collectEscapeEditsdid alastIndexOfplussliceback to the line start for each escape: O(n²) on a long line of escapes.micromark and mdast-util-to-markdown are at their latest releases, and upstream is only now starting on this class (an unreleased EditMap commit, 2026-09-11).
Latent second failure. At scale 8,
deep-blockquotes(4000 levels) anddeep-lists(1600 levels) are refused. The parent test counted only parsed classes as finished, so it would have failed on them next. Reproduced atPATHOLOGICAL_SCALE=2.1.2. Corpora soak: the designed refusal treated as a crash
8c6bc00c4made the nesting refusal a typedNestingTooDeepError, whichparseMarkdownwraps ascause. The OSS-Fuzz loop calledparseMarkdownbare. Reproduced locally ataf945bfb2: same error, 16 382 levels.App path.
TiptapEditorcaught the same refusal, logged it to the console, and kept an empty, editable document. The first keystroke flushed that empty document over the file just opened, and the user saw no message. A refused external change had the same hazard in reverse: the old content stayed on screen, and the next flush overwrote the new text.What changed
Pipeline (linear time, with trees identical to stock):
parser/fastPaths/: micromark constructs registered in every parse mode.]is consumed as data only when no unbalanced label start exists, found through an incremental opener index. A later]also cannot complete a gfm footnote call, because its label would contain a bare], which no definition label can.codeTextwould.patches/:pnpmpatches for micromark'sresolveAllText(one compacting pass) and mdast-util-to-markdown'ssafe()(an own-key check). A version bump of either package now fails install until the patch is remade, on purpose.serializerCosmetics.ts: the escape guard's line context is found in one forward pass. The code-range helpers moved toserializerCodeRanges.tsto keep the file under 300 lines.Soak and tests:
src/test/arbitraryMarkdown.ts: one shared helper returns a nesting refusal as data and rethrows anything else. It classifies throughnestingRefusal(), by type along the cause chain, never by message. The OSS-Fuzz loop and the pathological runner both use it. Only the input may be refused; re-parses of the pipeline's own output still throw.pathologicalScaling.test.ts(PR tier) asserts a CPU-time growth exponent per class, never a duration, so load cannot fail it (details under Validation).App:
services/editor/unparseableDocument.ts: a refused document puts the tab in Source mode through the existing forced-Source marker (it now records a reason:large-fileorunparseable) and shows a toast. The toast states the depth and the limit for a nesting refusal, and gives a general message otherwise.useTiptapFlushwrites nothing for a tab markedunparseable. That covers the pending keystroke, Save's flush, and the unmount flush.Audit
Codex CLI (
codex exec --sandbox read-only, high reasoning), three rounds.text.nullcatch-alls.isNestingTooDeep.*<TAB>then two lines of code spans).markdownSplitView. Split view is a window-wide layout the user chose. Lifting the Source-mode override restores it, and clearing it would change every tab in the window.safe(): identical output for 100 000 fuzzed calls.]re-index watch fails the scaling test (exponent 1.55).Validation
af945bfb2(clean worktree, frozen install):PATHOLOGICAL_SCALE=8 pnpm vitest run src/utils/markdownPipeline/__tests__/pathological/→ killed at 300 107 ms onlink-closers-with-openers, exit 1.pnpm test:soak -t "OSS-Fuzz"→NestingTooDeepError16 382 levels, exit 1.pnpm test:soak: 7 passed, exit 0. OSS-Fuzz leg: 1/500 refused (16 382 levels); 4/499 oscillate (0.8%, under 2%).FUZZ_RUNS=500 FUZZ_SEED=): passes, as on CI. See Not covered for what that green does and does not mean.pnpm check:predelta→ all 44 delta gates passed, exit 0.pnpm check:all→ exit 0 (app tier: 1 739 files, 39 839 tests; coverage 95.2 / 91.52 / 94.29 / 96.16).Not covered
rust-audit: the requiredrustcheck is expected to fail only on RUSTSEC-2026-0285 (rustls0.23.41 insrc-tauri/Cargo.lock). That is fixed separately onfix/rustls-rustsec-2026-0285. This PR does not touchCargo.lock.Still super-linear, measured, finishing well inside the bound:
nested-strong-emph(~5 s at scale 8): micromark re-resolves each matched span and splices per match.nested-brackets(~2 s): label-end serializes the label to look up a definition.*a*× 24 000, 96 KB: 6 s).resolveAllLineSuffixessplices, andmdast-util-find-and-replacedoesindexOfper text node.Removing these changes what the stock algorithms do rather than how fast they find it, or needs further dependency patches. None is in a failing soak class.
The content server (
server/content) builds its own remark chain. It gets the two patches but not the fast paths.The soak's editing fuzz is green by accident, and it hides real defects. Nothing in this PR causes them; they reproduce identically at
af945bfb2.soak.ymlsetsFUZZ_SEED: ${{ inputs.fuzz_seed }}, which is an empty string on scheduled runs.Number("")is 0, so the scheduled job has always run seed 0, not the documented default20260805.**/*runs merge on serialize and italic is lost (wordwor**wordword***# *).RangeError: Position 6 out of range.FUZZ_RUNS=500 pnpm vitest run src/test/editingFuzz.test.ts.