fix: run the soak fuzz's documented seed, and fix the editor bugs it hid - #1416
Merged
Merged
Conversation
…ion on save The weekly soak's editing fuzz (#1407) found italic typed next to bold lost on save. Emphasis and strong were both written with `*`, and flush `*` delimiters merge into one run: `wordwor**wordword***# *`. micromark decides whether a run opens or closes from the characters around the WHOLE run, which neither node looked at, so the italic came back as literal asterisks (fuzz seeds 20260805, 5 and 6). Emphasis flush against a strong sibling is now written `_`, which never merges with `**`. Everywhere else it stays `*`, so existing documents keep their spelling. The strikethrough handler had the same class of fault from the other side: it tested ASCII punctuation only, while micromark counts every Unicode P and S character, so `文字~~。word~~` reparsed as literal tildes. All three attention delimiters now go through one module with one flanking model: micromark's character classes, upstream's encoding table, and character references that always cover a whole code point.
mdast-util-to-markdown makes a non-flanking delimiter work by writing its neighbour as a character reference, one UTF-16 code unit at a time. Beside an emoji that emitted `�` plus a raw low surrogate, or `��` with a delimiter on each side, and the character decoded to U+FFFD. VMark repaired the string afterwards. That saved the emoji but not its neighbour: the delimiter beside the pair had already decided it flanked a raw surrogate, which micromark reads as a letter, and after the repair it sat against `&`, which is punctuation. The soak's editing fuzz lost an italic that way (#1407, seed 7: `wordword*🙂**# ***`). The encoding now covers the whole code point at the moment it happens, in the patched containerPhrasing and in VMark's own attention handlers, so every flanking decision sees the final text. The string repair is deleted, and its module with it. A property test now pins the class rather than the shapes: bold, italic and strike runs over letters, CJK, RTL, emoji, ASCII and full-width punctuation and spaces must round-trip. A 40,000-case probe over that alphabet now fails only on a strike mark over a whitespace-only run, which markdown cannot spell and markEdgeWhitespace.ts deliberately moves out; the property excludes that shape by construction and says why.
… paragraph CommonMark 5.2 lets a list interrupt a paragraph only if its first item does not start with a blank line and, when ordered, starts at 1. Upstream joins the children of a tight list item with no blank line, so an empty nested item written under a paragraph was read back as that paragraph's text: the soak's editing fuzz turned an empty nested ordered item into the characters "1." (#1407, seed 3: `1. **b**\n 1.`). A nested ordered list starting at 0 or 3 was lost the same way. A new join adds the blank line exactly when the pair would otherwise break, and keeps a longer captured blank-line run (ADR-1a). A list that can interrupt keeps its tight spelling, and an empty paragraph, which writes no text, never triggers it. Bullets follow the same rule, and that changes one pinned expectation. `- Parent text\n -` round-tripped in VMark only because the parser inserts the blank line itself on input (normalizeBareListMarkers); a CommonMark reader such as markdown-it renders it as `<h2>Parent text</h2>`. The serializer now writes `- Parent text\n\n -`, which both read as the nested empty item, and adapter.test.ts asserts that spelling and its stability. blankLinesJoin's parameter type gains `| undefined` so an mdast node satisfies it under exactOptionalPropertyTypes. The serializer header also names the attention and join modules it now coordinates with.
Tiptap core's clearDocument plugin turns a document emptied by select-all-and-delete back into a plain paragraph. Its test is "the old selection covered everything and the new document is empty", and an undo taken with the text fully selected passes it whenever it leaves an empty list item or heading behind, so the plugin lifted that item in an appended transaction. prosemirror-history files a transaction appended to an undo in the redo branch without remapping what is left in the undo branch. The first undo therefore showed a document that never existed (a paragraph where the empty list item was), and the next undo replayed steps recorded for a document that no longer existed: `RangeError: Position 6 out of range`. The soak's editing fuzz found it (#1407, seed 4). A new undoIntegrity extension tags every history transaction with the plugin's own `preventClearDocument` opt-out, in the extension dispatch hook that both the undo/redo commands and prosemirror-history's beforeinput handler go through. Select-all-and-delete keeps its normalization, and a test pins that.
The weekly soak never ran its documented seed. soak.yml passed
`FUZZ_SEED: ${{ inputs.fuzz_seed }}`, which is the EMPTY STRING on a
scheduled run, and editingFuzz.test.ts read
`Number(process.env.FUZZ_SEED ?? "20260805")`. `??` falls back on
null/undefined only and `Number("")` is 0, so every scheduled run fuzzed seed
0. At 20260805, and at seeds 3 to 7, the fuzz fails on the editor bugs fixed in
the preceding commits (#1407 found the mechanism).
Fixed as a class, in one pass:
- soak.yml falls back to 20260805 in the expression, and runs the fuzz with
the verbose reporter, because the default reporter prints no test names for
a passing file and the name is what carries the runs and seed.
- src/test/envInteger.ts reads an integer knob strictly: unset means the
default, anything set that is not an integer literal throws with the name
and value. All four numeric environment reads in the repo use it:
FUZZ_RUNS, FUZZ_SEED, FAST_PATH_SEED, and PATHOLOGICAL_SCALE, where an empty
value used to shrink every pathological input to the 4-character floor.
- Every other workflow input handed to a step was checked. release-smoke.yml
(twice) and update-homebrew.yml already handle an empty value on purpose, and
now say so with a reasoned `# input-empty-ok:` marker. release.yml pasted its
version input into the shell body; it now goes through env like the rest.
Two gates keep it fixed, both mutation-checked by reverting a fix and watching
them fail:
- check-workflow-input-fallbacks.test.mjs parses every workflow as YAML and
fails on an env value that reads an input with neither a fallback nor a
reasoned marker, and on any input interpolated into a run body.
- check-numeric-env-reads.test.mjs walks a TypeScript AST over the repo and
fails on Number/parseInt/parseFloat/unary plus applied to process.env.
The previous commit kept Tiptap's clearDocument plugin off history transactions by setting its opt-out meta. The branch's cross-model audit found a second plugin with the same mechanism: footnote cleanup deletes a definition whose last reference an edit removed, and an undo that removes a reference is such an edit. Editing an unreferenced definition, adding its reference, then undoing twice deleted the definition on the first undo and threw `RangeError: Position 18 out of range` on the second. Two instances make it a class, and the class belongs to the combination, not to either plugin: prosemirror-history files a transaction appended to an undo in the redo branch without remapping the undo branch, so ANY appendTransaction that changes the document in response to an undo corrupts what is left. undoIntegrity now records the history transaction being applied, in the extension dispatch hook every view dispatch passes through, and a filterTransaction refuses any other document-changing transaction appended while it applies. A transaction dispatched during that window comes through the hook as its own root and is not refused; stored marks and metadata carry no steps and pass. Tests pin both regressions and that the normalizations still run on the ordinary edits they exist for.
…s references The serializer wrote a text line ending raw. One is a soft break and reads back as itself, but a line ending that starts a paragraph, ends it, or sits beside another one makes a blank line, which is structure: the parser drops it at a paragraph's edge and splits the paragraph at one inside. Inside a list item the empty first line also meant the item started with a blank line, so a nested list could not interrupt the paragraph above it. The branch's cross-model audit reproduced that from `- b\n 1. x`, against the new list join, which only looked for an empty paragraph. The root cause is the raw line ending, so a small text handler now writes exactly those line endings as character references and leaves a lone soft break, LF or CRLF, raw. It is a post-pass on upstream's escaping rather than an `unsafe` pattern, because upstream's hard-break handler reads any line-ending pattern in scope as "no line endings here" and degraded every hard break to a space: the spec corpus caught that attempt on 32 examples. CommonMark example 39 (`foo bar`) was ledgered as exactly this defect ("entity-newline injection … one paragraph reparses as two blocks"). It now round-trips, and its six ledger entries are removed, as the ledger requires.
…tors The branch's cross-model audit reproduced misses in the two gates added for the soak seed defect. check-workflow-input-fallbacks.test.mjs matched `inputs.x` with one regex over the raw expression, so `inputs['fuzz_seed']` went unseen, `|| ''` counted as a fallback, and a string literal that mentioned `inputs.x` was a finding. String literals are now masked before matching, bracket keys are read, every input in an expression needs a non-empty fallback of its own, and a fallback that is another input is checked in its turn. check-numeric-env-reads.test.mjs skipped any file not containing the exact text `process.env`, and did not see through `process["env"]`, `as string`, `!` or `<string>`. It now prefilters on the word `process`, unwraps parentheses and type assertions, resolves string-keyed element access, and parses each file in its own dialect. Each gap has a self-test that failed before the change, and both gates were mutation-checked again: reverting the soak fallback, or the fuzz test's readIntegerEnv call, still turns them red.
…s applied Round 2 of the branch's cross-model audit: the undoIntegrity guard lives in the extension dispatch hook, so an undo applied with `state.apply` and never dispatched bypasses it. Applying the audit's footnote setup that way still deleted the definition on the first undo and threw on the second. ProseMirror's filterTransaction cannot tell a transaction appended in this batch from the next root transaction, so a central guard cannot cover that path; no VMark code applies transactions that way. What CAN hold on every path is the normalizers VMark owns declining to react to a history batch at all. plugins/shared/historyBatch.ts detects one, including a transaction appended to an undo, which a plugin receives in a later round without the undo itself. Footnote cleanup and blankLinesGuard, the two VMark appendTransaction plugins that change the document outside IME composition, now check it. blankLinesGuard had the same shape: an undo that restored a block reset its captured blank-line count. Tests apply the undo directly and fail without the checks. The footnote plugin's change fits inside its size baseline, which ratchets down 384 -> 381. undoIntegrity's header states the boundary, and two assembly comments that still described the clearDocument-only guard are corrected.
… fallbacks
Round 2 of the branch's cross-model audit found three more misses.
check-workflow-input-fallbacks.test.mjs accepted any `||` after an input, so
`format('{0}', inputs.seed, github.ref || '1')` passed with the seed still
empty, and `github['event']['inputs']['seed']` was not recognized. It now
reads each expression with a small parser for the Actions expression grammar
and asks whether an empty input can reach the RESULT: `a || b` leaks what `b`
leaks (or `a`, when `b` is itself empty), `&&` and function calls pass values
through, comparisons and boolean functions never do. Property paths are
resolved case-insensitively through dots and string keys, and an expression
the parser cannot read is a finding, not a pass.
check-numeric-env-reads.test.mjs only followed the left side of `??`, so
`Number(override ?? process.env.FUZZ_SEED)` passed. It now follows a value
through both sides of `??`, `||` and `&&`, string concatenation, conditional
branches and templates, stops at a function call, and also counts arithmetic
coercion (`x * 1`, `-x`, `x | 0`) as a conversion.
Every new case has a self-test that failed first; both gates still report no
findings on the repository.
Round 3 of the branch's cross-model audit confirmed the round-2 fixes and
found three more, each reproduced before fixing.
- Footnote cleanup's "this document has no footnotes" cache went stale when it
stood down on an undo that brought footnotes back: after replacing a document
with plain text and undoing, deleting the restored reference left its
definition orphaned (origin/main cleaned it up). A history batch now resets
the cache before returning. The regression test fails without the reset. The
plain two-statement form puts the file at 384 lines, so its size baseline
returns to origin/main's 384; the branch's net change to that baseline is
zero.
- check-workflow-input-fallbacks.test.mjs treated only an empty literal as an
empty fallback, so `inputs.seed || ('' || '')` and
`inputs.seed || format('{0}', '')` passed. A fallback now protects an input
only if it cannot evaluate to empty itself, decided recursively; a function
call is assumed able to return "".
- check-numeric-env-reads.test.mjs did not recognize `globalThis.Number`,
`globalThis.Number.parseInt` or `window.parseFloat`. A global-object
qualifier is now stripped before matching a converter.
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
The weekly soak's editing fuzz has never run its documented seed.
soak.ymlpassedFUZZ_SEED: ${{ inputs.fuzz_seed }}, which is the empty string on a scheduled run, and the test readNumber(process.env.FUZZ_SEED ?? "20260805").??does not fire on"", andNumber("")is0, so every scheduled soak fuzzed seed 0 and stayed green. The mechanism was found while fixing #1407.At the documented seed 20260805, and at seeds 3–7, the fuzz fails on real editor bugs: italic lost on save, an empty nested list item turned into text, and an undo that throws
RangeError. This PR fixes those bugs at their root, fixes the seed plumbing as a class, and adds gates so neither comes back silently.The branch went through three read-only Codex audit rounds; every finding was fixed with a failing test first (see Audit).
Policy Gates (Required)
Linked Issue
Found while fixing #1407 (closed). No separate issue.
Type of Change
Root causes
Seed plumbing.
inputs.*expands to""on a scheduled run, andNumber(process.env.X ?? default)turns""into0. The same shape was inFUZZ_RUNS,FAST_PATH_SEEDandPATHOLOGICAL_SCALE(an empty scale shrank every pathological input to the 4-character floor). Inrelease.yml, an input was pasted into arun:body.1. Italic lost beside bold (seeds 20260805, 5, 6). Emphasis and strong were both written with
*, and flush*delimiters merge into one run, e.g.wordwor**wordword***# *. micromark decides whether a run opens or closes from the characters around the whole run, which neither node looked at.2. Italic lost beside an emoji (seed 7). mdast-util-to-markdown character-references a delimiter's neighbour one UTF-16 code unit at a time. VMark then repaired the split surrogate in the string afterwards, but that repair changed the neighbour from a letter (the raw surrogate) to
&, which is punctuation, after the delimiter beside it had already decided how it flanks. A delimiter on each side of one emoji produced��and destroyed the emoji outright.3. Strikethrough lost beside CJK punctuation (same class, found by a class-level probe).
handleDeletecounted only ASCII punctuation, while micromark counts all Unicode P and S, so文字~~。word~~came back with literal tildes.4. Empty nested list item turned into text (seed 3). CommonMark §5.2: a list may interrupt a paragraph only if its first item does not start with a blank line and, when ordered, starts at 1. Upstream joins a tight item's children with no blank line, so
1. **b**\n 1.reparsed as the text "1.". The same happened to a nested list starting at 0 or 3, and to text whose first character is an encoded line ending ( ), which the serializer wrote raw.5. Undo corrupts the history (seed 4). prosemirror-history files a transaction appended to an undo in the redo branch without remapping what remains in the undo branch. Tiptap core's
clearDocument(on "whole document selected, now empty") and VMark's footnote cleanup (on "reference removed") both appended document changes in response to an undo. The first undo showed a document that never existed, and the next one threwRangeError: Position N out of range.What Changed
serializerAttention.ts(new) owns theemphasis,stronganddeletehandlers with one flanking model: micromark's character classes, upstream's encoding table, and references covering whole code points. Emphasis flush against astrongsibling is written_; everywhere else it stays*.serializerStrikethrough.tsand its string repair are deleted.patches/mdast-util-to-markdown@2.1.2.patch(extends fix: linear-time hostile markdown and handled nesting refusals (fixes #1407) #1415's patch):containerPhrasingencodes neighbours by code point. The lockfile changes only by the patch hash.listInterruptJoin.ts(new) inserts the blank line CommonMark requires and keeps a longer captured blank-line run (ADR-1a).serializerText.ts(new) writes a text line ending that would make a blank line as a character reference; a lone soft break stays raw.plugins/undoIntegrity(new extension) refuses any document change appended to an undo or redo, via the extensiondispatchTransactionhook.plugins/shared/historyBatch.tslets VMark's own normalizers (footnote cleanup,blankLinesGuard) stand down on a history batch on any application path.soak.ymlfalls back to20260805and runs the fuzz with the verbose reporter, so the log names the seed.src/test/envInteger.tsreads integer knobs strictly (unset means the default; anything else must be an integer, or it throws with the name and value), and all four numeric env reads use it.release.ymlpasses its input throughenv:, andrelease-smoke.ymlandupdate-homebrew.ymlstate why their empty input is handled.scripts/check-workflow-input-fallbacks.test.mjsparses every workflow's expressions and fails on an input that can reach a step as""without a reasoned# input-empty-ok:marker, or that is pasted intorun:.scripts/check-numeric-env-reads.test.mjswalks a TypeScript AST and fails on any numeric conversion ofprocess.env. Both were mutation-checked: reverting the soak fallback or the fuzz's reader turns them red.attentionRoundtrip.property.test.tspins the attention class (letters, CJK, RTL, emoji, ASCII and full-width punctuation, spaces, all mark combinations). A 20,000-case probe of that shape failed 1,034 times before these fixes; the fuzz-reachable alphabet now fails only on the declared whitespace-only-strike shape (see Known, out of scope).cm-39entries ("entity-newline injection") are removed because the example now round-trips.Deliberate behaviour change (maintainer-approved)
- Parent text\n -(an empty nested bullet item) is now written- Parent text\n\n -. The tight form round-tripped only inside VMark, because its parser inserts that blank line itself on input (normalizeBareListMarkers); a CommonMark reader such as markdown-it renders it as<h2>Parent text</h2>.adapter.test.tspinned the tight form, and now pins the portable one and its stability. The maintainer chose the portable form for bullet and ordered lists alike, over limiting the rule to ordered lists.Audit
Three read-only
codex exec --sandbox read-onlyrounds overgit diff origin/main...HEAD: text empties a list item's first line; bracket access and empty fallbacks escape the workflow gate; spacing, bracket and assertion forms escape the numeric gate51f9e730a,b12e5c3c3,687216ab1)||satisfies the workflow gate; the right operand of??is ignoredaf8476184,869ab12d9). The dispatch boundary is documented, and VMark's normalizers stand down on any pathglobalThis.Number39d8b2f2e), each with a failing test firstThe round-3 fixes were not re-verified by Codex (the loop caps at three rounds). Codex could not run Vitest in its read-only sandbox, so every round verified with in-memory Node checks.
Known boundary of the undo guard: an undo applied with
state.applyand never dispatched is still exposed to Tiptap's ownclearDocument. No VMark code applies transactions that way; VMark's normalizers stand down on every path; the boundary is written in the guard's header.Validation
Seed matrix,
FUZZ_RUNS=500 pnpm vitest run src/test/editingFuzz.test.ts --reporter=verbose, on39d8b2f2e:Gates, on
39d8b2f2e(branch already containsmainat7c01cef1e):pnpm check:predelta: exit 0, all 44 delta gates passed.pnpm check:all: exit 0 (1,747 app test files and 39,906 tests, 93 gate test files, server suites, build and size limits).687216ab1. Seeds 10, 15, 42 and 100 fail before and after this PR (see Known, out of scope).Known, out of scope (distinct bugs, not fixed here)
-\n\n wordwordword: an item that starts with an empty paragraph followed by a block. The text lands outside the list, in micromark and markdown-it alike.-\n\n\n\n-: two lists separated only by an empty paragraph merge into one in VMark (markdown-it keeps two).-\n -\n - word: micromark flattens the nested list (markdown-it nests it).[{"kind":"text","text":"* "},{"kind":"text","text":"word"},{"kind":"enter"},{"kind":"text","text":"word"},{"kind":"text","text":"word"},{"kind":"text","text":"word"},{"kind":"select","a":38,"b":0},{"kind":"enter"}]FUZZ_RUNS=500 FUZZ_SEED=15 pnpm vitest run src/test/editingFuzz.test.ts.~beside emphasis in VMark's parser.\~*** ***parses as literal text in VMark but as<em><strong> </strong></em>in markdown-it, while\#*** ***is right in both; likely the~sub~dialect. The editing fuzz cannot type~. A 20,000-case probe with~in the alphabet failed 1,126 times.