Repository navigation
Fix list round trips, page order and sidebar drag and drop at the root, and add Duplicate (supersedes #42) - #52
Merged
Conversation
Reimplementation of the problems reported in #42, wider and at the root, with a compatibility harness so existing pages only improve. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tation plan Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ar/duplicate plan Reorders the heal migration before placement, adds in-transaction placement with a deadlock-free lock order, the cache seq guard and quiet task sync for the one-time refresh, the duplicate cleanup guard, and the projection's rendered-slot no-op rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… serializer Freeze today's serializer, importer and task extractor as knot_markdown::legacy (feature legacy-compat, enabled only for this crate's tests through a self dev-dependency; P20), and add the classifiers in legacy::compat: schema_violations, healthy, normalised, and the empty export_class/import_class tables later tasks fill. tests/compat.rs compares new against legacy over a corpus of about 19,000 documents (deterministic combinations of every node and mark nested up to four deep, in editor and import representations, plus 50 seeds of random mixed-representation documents with damaged and unknown content) and over the fixtures, the 40 PR #42 probe inputs (tests/probes, P21) and every healthy corpus export. New and legacy code are identical, so it passes; legacy_import_produces_schema_violations_today documents bug A1. round_trip.rs gains round_trip_fixture_directory; existing tests unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…m the third plan check Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…mport pulldown-cmark leaves out the paragraph tags of a tight list item, and the importer patched that per event type, missing hard and soft breaks, images and HTML. A line break in a list item (Shift+Enter in the editor) therefore imported as list_item[paragraph, hard_break, paragraph], which the schema forbids and the serializer can't export (A1). A TightParagraphs adapter now wraps the parser: it opens a paragraph on the first inline event directly under an Item and closes it before the next block-level event or End(Item); TaskListMarker passes through. It matches every pulldown event, tag and tag end by name, so a new one fails to compile instead of reintroducing the bug. With it: - hard and soft breaks stay in their paragraph (A1); - images and boards in tight items are promoted and kept (A2); - an HTML block between item lines no longer merges them into "ab" (A3); - an empty item, or one starting with another block, gets an empty paragraph first (A7, import side); - a rule is written into its list item or blockquote instead of the top level (A9, import side). The list_item special cases in Text and Code and ensure_para_child are gone. Import classes for the differential harness: markdown_escaping and leading_hard_break (text the writer can't express yet, a follow-up), item_first_child_not_paragraph, rule_in_container and html_between_item_lines. New tests: tests/import_lists.rs, new_import_is_schema_valid_on_every_input and import_classes_name_their_inputs, and the fixed-point fixture list_breaks.md. No existing test changed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Healthy inputs that imported differently with no named class (review I1): - A run in a tight list item with no content (only inline HTML, empty links, or images that share a line) no longer gets a synthetic empty paragraph. That matches the legacy importer. The empty paragraph exported as a blank line, which the next import dropped. - New import class image_after_block_in_tight_item (A2): a lone image or board as its own run after another block in a tight item. Legacy dropped it; the new importer keeps it. - Text that pulldown-cmark emits inside an HTML block (the block's extra indentation) is ignored with the block. It no longer lands directly in the list item, which broke the schema. Six probes are added. Task 2's own tests import_classes_name_their_inputs and image_and_board_in_tight_items_are_kept gain cases. No test that predates this plan changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…trip Nested content of a list item is indented by the marker's width only (A4), and the GFM checkbox is item content, written for ordered items too (A8). List items and blockquotes share one container writer that puts a blank line between two blocks exactly where Markdown would read the second as part of the first (A5, A6, A9): a paragraph line followed by a paragraph, an image or a list that can't interrupt a paragraph; `---` under a paragraph line; a paragraph, image or table after a list, quote or table; a quote after a quote. Everything that round-trips today keeps its bytes: a quote's paragraph followed by a paragraph, image, board or attachment (P2), empty items with their trailing space (P2), and blocks that end or interrupt the block above (headings, code blocks, tables, and lists at any number after a list, quote or table). Table cells keep the legacy layout, since nothing in a cell reads back as structure. A task item that doesn't start with a paragraph (only the old importer stored one) gets its checkbox on a line of its own, so the checkbox no longer turns the block into text. Compat harness: export classes nested_tasklist, multi_block_item, quote_block_sequence, ordered_checkbox and rule_in_container, each an independent model of how Markdown misread legacy's output; new tests import_export_import_is_stable, export_classes_hold_the_fixed_point and export_classes_name_their_documents. Controller rulings folded in: - the corpus's pair_blocks gains p-hard-break, so the healthy A1 editor shape sits in every container under the byte-identical differential; - markdown_escaping and leading_hard_break move from import_class to legacy::compat::stability_exempt, used only by the stability test. import_classes_name_their_inputs (added earlier on this branch) loses those cases, which move unchanged to the new stability_exemptions_name_their_inputs: a mechanical adaptation the ruling requires. knot-server enables knot-markdown's legacy-compat feature as a dev-dependency, for the schema check in markdown_import_integration. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A paragraph, image or table right below a nested list or quote only goes on it when its last line is still open: a paragraph line or a table row. The writer now records that per written line, so a list or quote ending in a heading, a code block, an empty item or a quote's blank `>` line is followed without a blank line, as legacy wrote it. Eight healthy shapes that round-trip losslessly today had gained a blank line (for example a nested step list whose last step is a code block, then a closing paragraph); they keep their bytes again. The compat model gets the same `ends_open` flag, and the corpus now nests lists and quotes two levels deep, so the byte-identity differential meets these shapes. New guard export_classes_name_only_damaged_documents: every healthy document an export class names must be one legacy damaged (its legacy round trip is lossy, or the new importer reads legacy's bytes differently from the new ones). The byte-identity differential skips classed documents, so this is what keeps a class from hiding a change to a lossless page. No exemption was needed: random/14/91 (literal "> quote" text) is lossy under legacy. Correction to 0a74b70's message. It said everything that round-trips today keeps its bytes, and called the export classes an independent model of how Markdown misread legacy's output. Neither held: the writer changed the eight lossless shapes above, and the classes mirror the writer's blank-line table, so they named exactly those shapes and the differential skipped them. The classes still mirror the writer; their precision is now checked by the guard above instead. An item the old importer stored without its first paragraph, whose content has a blank line between two of its lines, writes its marker on a line of its own, as the empty paragraph the importer adds back does. Its export is then a fixed point (`- \n > q\n\n > q`). Legacy indented every line of an item, so such an item never exported as before; one without blank lines (`- ## h`) keeps its bytes. export_classes_hold_the_fixed_point no longer skips these schema-invalid documents. Review minors: the compat model now takes a list's first written item for whether the list interrupts a paragraph, as the writer does (an item without blocks or checkbox is written as nothing), and a shared list_items helper gives write_block and write_cell_block their item markers. Tests changed that this branch added (none from main): export_classes_name_their_documents keeps its cases, moved into class_cases() for the guard to share, plus new ones; export_classes_hold_the_fixed_point drops its schema filter; the corpus gains nested_endings(). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…the page Pages the old importer damaged (text and hard breaks directly in a list item, quote, cell or the top level) and pages with nodes the schema doesn't name failed every export: GET markdown, the cache, history restore and from-template all returned 500 (A1, A10). - normalise::block_children applies the glue rule (P3): a chain `paragraph (run paragraph)+` is one paragraph, any other run of stray inline nodes a paragraph of its own. Every block container of the serializer goes through it. In a glued paragraph, text with equal written marks is one run, and hard breaks at its edges are left out; a run of nothing but hard breaks is not written. - Unknown nodes are written as their text (P4); serialise_with_report names them, serialise keeps its signature. The server warns and counts knot_markdown_unknown_nodes_total on every refresh. - knot-export's walk applies the same glue rule (its own copy: knot-export doesn't depend on knot-markdown); both crates' tests share the vectors. text_under_blockquote_is_wrapped stays as it was. - An empty first paragraph in front of a block an item can start with is written as the item without it (`- ## h`), so the item the importer reads back from `- ## h` exports the same again; an item starting with a rule gets its marker on a line of its own (`- ---` is a rule). - compat: export class damaged_shape. The frozen legacy copies call snapshots of the crate helpers they used (mark table, board sentinel), so the reference is fully frozen. Tests added on this branch (none from main changed): - compat.rs corpus_docs() also holds the legacy import of every fixture and probe (ruling a). new_import_is_schema_valid_on_every_input also runs over the legacy export of every corpus document (ruling d), with the empty-blockquote violation of Task 4b's lone `>` bug as a named, must- still-occur exemption. no_text_is_lost now requires every export to succeed: the tolerant export never fails. - New new_export_is_a_fixed_point (ruling b, serializer rule 2): each non-fixed point must be explained by a named class with a repair that takes its cause out; 13 classes, counts in the doc comment. - common::read (the inverse of build) and PartialEq on N and Run, for the repairs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ss floors Self-review of 713f89e: the comment on the floor constants still gave the export count from before the tolerant export (21300); every corpus document now exports (23700), 13000 of them fixed points (MIN_FIXED_POINTS). Comment only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… import Four pre-existing importer bugs lost content on every pass of a page through Markdown (history restore, from-template, zip and Markdown import). Each is fixed at its root in from_markdown.rs: - Code blocks: pulldown-cmark emits a code block's text line by line in a list item or blockquote (and in an indented block), and the importer stripped the line break of every piece, so "x\n\ny" became "xy". The text now goes in whole; only its last line break, the fence's, is removed at the end of the block. The writer was right. - A blockquote without blocks (a lone ">", or a quote of only an HTML block) now gets an empty paragraph, as `blockquote: block+` requires. It exports as ">" again. - Table cells: a GFM cell is one line, onto which the writer joins the cell's blocks with a space. The importer now splits that line back into blocks at each image or board, trimming the joining spaces, so images and boards in cells, alone or next to text, are kept instead of dropped. - Nothing in a cell is a promotion candidate any more, and a cell's start and end clear the candidate state, so a cell's image is no longer handed to the next lone-image paragraph in place of its own. The paragraph promotion and the cell split share the image and board attribute rules (ImageBlock). Compat harness: new named import classes code_block_line_breaks (renamed from the brief's code_block_in_container: indented code blocks at the top level lost their line breaks too), empty_blockquote, cell_image_candidate_leak and image_in_cell (renamed from cell_inline_image: lone cell images were dropped as well), each with a naming test, and a probe per bug. No other input changes its import. The Task 4b hand-offs in tests/compat.rs (added on this branch, not on main) go: known_violation, the fixed-point classes code_block_in_container, image_in_cell and empty_blockquote, and the stability exclusion loses_content_outside_lists. The empty_paragraph repair now gives an emptied quote its paragraph, as the importer does, and a new fixed-point class indented_image_in_cell names the one remaining writer limit: an image in a list item in a cell sits next to the item's indentation on the cell's line, which the split reads as the one joining space. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…oggles /tasks listed the tasks it read back from the page's Markdown, while its toggles flip the Nth `list_item` with a `checked` attribute in the Y.Doc. The two disagreed (spec A11, A12): a nested checklist lost its parent, so the child was listed at the parent's toggle index; checklists in table cells weren't listed but were counted; a plain bullet starting with `[ ] ` was listed but not counted. - knot-crdt: new `tasks` module with `task_items` (pre-order, every `list_item` with a `checked` attribute, table cells and ordered lists included) and `patch_task_checked`, moved from room.rs and built on it. room.rs calls it unchanged; its existing tests pass as they are. - knot-markdown: `extract_tasks_from_doc` lists one task per task item: index and checked state from the walk, text, assignee and due date from today's `extract_tasks` (unchanged) over `task_item_markdown`, `- [ ] ` and the item's own blocks written by the new serializer, without the blocks that are or hold a list item (P1). - knot-server: `refresh_markdown_inner` indexes tasks with it. Tests: - tasks_compat.rs: unit cases (nested checklist, table cells, ordered lists, a bracketed plain bullet, a due date in a second paragraph, empty items); the lockstep property over the whole corpus (21,452 toggles); and the differential against legacy (legacy export, then extract_tasks) on pages where legacy agrees with the room. - tasks_lockstep.rs: a toggle with the listed index flips exactly the listed item, for a nested checklist, a checklist in a table cell, and an ordered list (regression guard, fixed by Task 3). The differential needed more named task classes than P1's `task_with_nested_list`, each a legacy writer bug that made its listing read other content as the item's text (`legacy::compat::task_class`): `task_block_syntax_as_text` and `task_lines_read_as_code` (A4: block syntax after the checkbox read as text, or lines below a blank line read as an indented code block) and `block_read_into_task` (A5, A6: the block below the list continues the item's paragraph). A remaining difference must vanish once text that reads as Markdown syntax is taken out of the page, the writer's escaping follow-up (9 corpus tasks). No existing test changed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`extract_tasks_from_doc` is public, and its doc linked to the crate-private `task_item_markdown`, which rustdoc warns about (rustdoc::private_intra_doc_links). Name it as code instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A task line that read as block syntax lost the task its text. A paragraph line `1. slides` or `- milk` (after Shift+Enter, or as a second paragraph) was read back as a nested list item, which took the task's place: /tasks listed it without text, assignee or due date, where main listed "Prepare: 1. slides". task_item_markdown now writes the item's text so that it reads as the text it is (controller ruling). This Markdown is never stored or exported, so the export keeps every byte: - a line right below a line of text goes on that paragraph, indented past any block syntax, as legacy wrote it; a paragraph right below another goes on the same way; - a line that starts a paragraph elsewhere (below a heading, rule, table or quote, in a quote, after a blank line) gets a backslash before a list marker, heading, quote, fence, rule, HTML block or link definition; - a line break in a heading starts such a paragraph, and is escaped too; - no blank line goes right below a checkbox without text, which ended the item before its content; - code is fenced longer than any run of backticks in it. The differential's escaping escape hatch is gone: no task needs it now. The three extra task classes (A4, A5, A6) are ratified with a precision guard: every classed task listed otherwise than legacy listed it must be one legacy damaged (its page's legacy round trip changes the item), so a classed task legacy writes back as it is lists as before. within_class now requires a new assignee or due date, where legacy found none, to come from the item's own links. The test reuses compat::holds_list_item, and the task_items unit test gains a blockquote. New tests: an item with text of its own is never listed without text (over the corpus), the list-marker cases list as main did, and heading-first and code-first items keep their checkbox. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Zip export, public shares and search read doc_markdown_cache whatever its age, and task rows come from the last reindex, so a page nobody edits after this upgrade would keep its broken legacy render and task rows (P17). - Migration: doc_markdown_cache.renderer_version (default 1, so every existing row is stale) and a BEFORE INSERT OR UPDATE trigger that stamps knot.renderer_version when the writer sets it, gives an UPDATE without it 1 (old pods during a rolling upgrade, the kept put), and keeps an INSERT's own value. - MarkdownCacheStore::put_rendered sets the version in its transaction and only writes when no newer seq is cached; it reports whether it wrote, and the refresh skips the task reindex when it didn't. stale_ids lists live pages with no row or an older version, by d.id after a cursor. - TaskStore::sync_for_doc_quiet writes only the task rows whose values change, so a healthy page's rows keep updated_at and /tasks doesn't reshuffle. upsert_for_doc behaves as before. - Rooms::get_active and current_snapshot (P18): the refresh uses a room's state when one is running and otherwise loads the stored state, so neither it nor the sweep boots rooms (they are never evicted). - reindex::spawn_with_sweep (now used by main) also takes ten stale pages per tick, in id order after a cursor, and refreshes them quietly. One replica sweeps at a time, holding an advisory lock on a dedicated connection for the whole pass; after an empty pass it rests ten minutes. reindex::spawn is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The case proves that a stale page with a running room is refreshed and keeps its room, not which snapshot path the refresh took. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- export_markdown_only (from-template) stamps the cache row with the stale version (seq guard kept), since it leaves the template's task rows as they are; the sweep then refreshes the template in full. - A refresh whose task write (or the doc lookup it needs) fails marks the cache row stale again (new MarkdownCacheStore::mark_stale), so the sweep tries the page again. - export_check::hydrate delegates to stored_snapshot: one load path. - stale_ids runs as an unnamed statement (.persistent(false)), so the cursor stays an index condition instead of a generic-plan filter. - The sweep pings its lock connection each tick and ends the pass when the database dropped it. - RefreshError loses NoRooms and Acquire, which no refresh produces; live_snapshot gets its own LiveSnapshotError. - refresh_sweep.rs: tests for each of the above, for a refresh older than the cache skipping the reindex, for an id both swept and edited taking the edit path, and for a partly changed page keeping its unchanged task rows' timestamps. The tests use RENDERER_VERSION and STALE_RENDERER_VERSION instead of literals, and wait for the task rows themselves: a refresh writes them after the cache row, which made the round-1 assertions racy (1 failure in 20 runs). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The spec's real-data check (Compatibility guarantees, 2): what upgrading a database to this release changes, found without changing anything. compat_report reads every live page from storage the way the refresh sweep loads it (export_check::hydrate, no room booted, P18), and reports: - the Markdown export by the legacy and the new serializer, by outcome: unchanged, an export class from knot_markdown::legacy::compat, not_healthy, or healthy_changed. Each outcome comes with the first page's diff; - the /tasks listing by legacy (legacy export, then extract_tasks) and by extract_tasks_from_doc, using the task classes and the tasks_compat.rs rule for what a class may change. On pages where the legacy listing agrees with the room, a task outside the classes listed otherwise is listed_differently; - the sort-key groups the heal migration would rewrite, from a SELECT mirroring P7's predicate: an invalid, over-64 or repeated key, and every group of a workspace's root list (top level plus template children) when one of them needs it; - the stale pages the sweep would render again: the sweep's own stale_ids, or every live page when doc_markdown_cache has no renderer_version (a backup from before the upgrade). real_data_compat (ignored) runs it against KNOT_COMPAT_DATABASE_URL over a read-only connection (default_transaction_read_only) with no migrations. It prints the report and fails on healthy_changed, listed_differently or a panic. Two seeded tests run it: a healthy page and a damaged page on the current schema (1 unchanged, 1 damaged_shape; the connection refuses writes), and a database migrated only up to the release before. The second is beyond the brief's two tests, because the check must work on a pre-upgrade backup: all pages stale, and the four heal reasons including the root-list rule. The Helm README says how to run it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The real-data check now fails when a page doesn't load (load_failed joins the failing outcomes), as knot-server export-check does. A database the check can't read (a missing privilege or column) or a page whose state doesn't decode was not compared, so it must not pass. The report keeps the failing pages, counts each page once, and lists them. New test: a_page_that_does_not_load_fails_the_check. - The task differential (Listed, the listings, the agreement rule, the per-task verdict, within_class, own_runs, USER_PREFIX/TIME_PREFIX) moves into knot_markdown::legacy::compat, and both tests/tasks_compat.rs and the real-data check call it, so the gate judges pages by the rule the harness proves on the corpus. For tasks_compat.rs this is a mechanical move: its test bodies are unchanged. New test: a_classed_task_is_counted_under_its_class. - The README sentence on when the check fails names unreadable pages. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…sk listing A digit marker that starts a paragraph of a task's text, below a heading, rule, table or quote, as a quote's first line, after a blank line or right below a checkbox without text, listed as `1 . slides` (`h 2 ) two`). Main listed `1. slides`. The `/tasks` Markdown now escapes such a marker with an empty inline HTML tag in front of it, which extract_tasks leaves out, instead of a backslash before the `.`, which split the text that extract_tasks joins with a space. Only task_item_markdown writes it; no export changes. The escape decided before write_item drains the blank lines below a lone checkbox is documented: it stays right, because Markdown goes on with no paragraph after a checkbox without text, so that line starts a paragraph either way. A setext-underline case pins that. The other carried review items were already done: the empty-first-item model (a2362cf), the shared list_items helper for write_cell_block (a2362cf), and the within_class tightening, the escape-hatch ceiling, the shared holds_list_item and prefixes, the blockquote case in the task_items unit test and the TaskListMarker probe for heading- and code-first items (1815228). Tests changed, all added on this branch (Task 5), not from main: in block_syntax_starting_a_paragraph_below_another_block_keeps_the_task, the pins "1 . x", "a h 1 . slides" and "h 2 ) two" encoded the split and now read "1. x", "a h 1. slides" and "h 2) two"; the counts comment records 2,384 classed tasks listed otherwise (was 2,385). Floors are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and the real-data check The leftover minors of the Task 6, 7 and 7b reviews (Task 7c). Refresh worker: - The sweep's per-tick ping on its lock connection is bounded to 5 s (LOCK_CHECK_TIMEOUT). It runs in the worker's only loop, before the refresh of edited pages, so a half-open TCP connection could hold every edit's /tasks refresh for about 15 minutes. A ping that fails or times out ends the pass, and the connection is dropped, not closed. Unit tests inject the ping future. Real-data check (tests/markdown_compat.rs): - A page that legacy exports and the new serializer can't fails the check (no_longer_exports), whatever its class or health. - A new listing with another number of tasks than the room has task items fails the check (lockstep_broken). Before, such a page could pass as no_tasks or no_legacy_listing, or panic inside task_verdicts. - The export and task decisions are pure functions (classify_export, classify_tasks), and their decision tables are unit-tested, including that every failing outcome makes violations() > 0. - The sort-key SELECT is pinned on a pre-upgrade database with keys ending in 0, an empty key, a bad group under a template, archived rows in a bad group outside the root list, and a healthy second workspace. - The operator README names the two new failing outcomes. Task listing tests and wording: - Comments and a test name say "legacy" for the frozen reference, not "main", and drop the branch history (to_markdown.rs, tasks_compat.rs). - The digit-marker test no longer repeats the two cases below a heading that block_syntax_starting_a_paragraph_below_another_block_keeps_the_task already pins. - New escape edge cases: a bare marker, a tab after the marker, and lines below an escaped marker (no HTML block swallows them). - legacy/compat.rs gets a plain module doc. No exported or stored Markdown changes. tasks_compat.rs and markdown_compat.rs were added on this branch, so P6 doesn't apply to them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Self-review of Task 7c. The Sweep::next doc now names the unanswered check, besides a lost connection. The Sweep.lock doc keeps the two-replica note next to the check it refers to, and gives the 5 s bound on edited pages a paragraph of its own. Docs only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The old generator decremented "1" to "0", below which nothing sorts, so
the 7th "New page" at a level got "0m" after "0" and the 8th got "0m"
again: 409 doc.sort_key_conflict under a parent, a silent duplicate at
the top level. When a was a prefix of b it ignored b's suffix
(between("m", "m01") == "m0h"), so inserting after the same page
returned its neighbour on the 7th try. Every append added a character.
lexorank now uses the fractional-index midpoint over [0-9a-z]:
- between(a, b) -> Result<String, KeyError> validates both neighbours
against ^[0-9a-z]*[1-9a-z]$ and their order, never panics, and never
emits a trailing '0'. between(None, None) stays "m".
- is_valid(k) is the validity rule; spread(n) gives n evenly spaced keys
(the formula the heal migration's SQL reproduces).
- sort_key_between keeps returning String (P5): it unwraps between, and
restore's top-level path calls it. Its call sites are unchanged.
New tests: tests/sort_key_props.rs (all six fail on main),
placement_integration.rs (all three fail on main: 409 on the 8th child,
duplicate top-level keys, 409 on the 7th insert-after), and lexorank's
invalid_inputs_are_errors_not_panics and spread_is_sorted_valid_distinct.
Existing tests changed (P6): lexorank's in-module tests take .unwrap() on
between's results and check_between uses is_valid instead of the removed
is_base36. Both are mechanical adaptations to the Result API.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ys unique The old generator wrote keys ending in '0' and repeated ones, and the old UNIQUE (workspace_id, parent_id, sort_key) treated NULL parents as distinct, so top-level keys could repeat and their order was undefined. Migration 20261024120000_heal_sort_keys.sql (P7), in its migration transaction: - drops the old constraint; - rewrites every sibling group, archived rows included, that holds an invalid key (not ^[0-9a-z]*[1-9a-z]$), one over 64 characters, or a repeated one. A workspace's root list (top-level rows plus the children of templates, which the sidebar shows at the top level) is respread as one list, so the combined order users see is kept; - ranks rows by sort_key COLLATE "C", created_at, id (the sidebar sorts by code unit) and gives a list of n rows lexorank::spread(n), computed by pg_temp.knot_spread_key, which is dropped at the end. Only sort_key changes; updated_at stays; - adds documents_sort_key_unique UNIQUE NULLS NOT DISTINCT (workspace_id, parent_id, sort_key) DEFERRABLE INITIALLY IMMEDIATE, so one UPDATE can rewrite a whole group. Re-running the file changes nothing. markdown_compat.rs's read-only SORT_KEY_GROUPS and the migration now point at each other. DocStore::siblings and list_alive order by sort_key COLLATE "C", id (list_alive keeps parent_id NULLS FIRST first), so a database whose collation is a locale returns the sidebar's order. Tests: migrations_apply.rs (healthy groups untouched, order kept and keys fixed, exact spread(n) keys, the root list with template children, an idempotent re-run, top-level duplicates rejected, and a one-statement key swap), and documents.rs siblings_and_list_alive_sort_by_code_unit. The order tests put an ICU collation on sort_key, because the test server compares by byte. Existing tests changed because they insert duplicate top-level keys, which the new constraint rejects (P6); each now uses distinct keys: - knot-storage documents.rs templates_flow_set_and_list; - shares_integration.rs cross_doc_revoke_returns_404_and_token_stays_valid and public_board_svg_404_for_cross_doc_board; - comments_integration.rs cross_doc_comment_mutation_is_rejected; - search_integration.rs make_doc, now after the last top-level page, which fixes results_capped_at_max_limit and multi_word_prefix_is_and. Not listed in P6, same reason: knot-storage documents.rs purge_takes_rows_stamped_with_the_root_outside_its_subtree. Its raw UPDATE moved a child keyed "m" to the top level beside its parent, also "m"; the child is now keyed "n". Also: the doc comment of ten_new_top_level_pages_get_distinct_ordered_keys now says what main did (from the 7th page on, every new page got "0m"). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The heal ranks a root list by sort_key COLLATE "C", parent_id NULLS FIRST, created_at, id: the sidebar's own tie-break for keys shared across groups (list_alive lists top-level pages first, then template children by parent, and buildTree's sort by key is stable). Within a group the parent is constant, so per-group order is unchanged. The root_list test oracle orders the same way. New test heal_ranks_tied_root_keys_as_the_sidebar_does: template children older than a top-level page with the same key keep the rendered order, checked against list_alive plus a stable sort as buildTree does. - heal_assigns_exactly_lexorank_spread_keys also covers n = 700 (key width 3) and checks the longest key per size. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…of failing
The routes read a page's siblings outside any transaction and handed
`sort_key_between` whatever keys they found. Two "New page" requests at
once computed the same key and one failed with 409; a key written by an
old release or kept from before the heal ("0", "m0", "M", a 70-character
key) panicked the request; two moves that swap nesting could both succeed
and leave two pages each other's parent.
New `knot_storage::placement`:
- `Placement { First, Last, After(id), Before(id) }`.
- `lock_group` (P8): at the top level a transaction-scoped advisory lock
(1802399604, hashtext(workspace_id::text)); under a parent the parent's
row FOR NO KEY UPDATE, which also refuses an archived parent and replaces
`lock_live_parent` on the placed paths. Re-taking it is a no-op.
- `lock_move` (P8): the target parent and the moved row in one id-ordered
FOR NO KEY UPDATE statement; at the top level the advisory lock, then the
row. The cycle check runs after it.
- `place`: locks the group, reads it whole (archived and templates
included, the moving page excluded) by `sort_key COLLATE "C", id`,
requires a live, non-template anchor (else `BadAnchor`), and asks
`between` for a key next to the anchor's immediate neighbour in the whole
group. On an error or a key over 64 characters it respreads the group
with `spread(n)` in one UPDATE (P7) and computes the key again. A moving
page inside the group gets the provisional key '~' || id in the same
statement. The UPDATE reads its rows from unnest($ids, $keys) rather
than a VALUES list: the same single statement, with no SQL built at run
time and no bind-parameter limit on large groups.
- `retry_on_deadlock`: runs a transaction once more on SQLSTATE 40P01.
`DocStore` gains `create_placed` and `move_placed`, and `DocStoreError`
gains `BadAnchor`. Both, and `restore`, retry once on a deadlock. Restore
takes the top-level group lock before its parent's row when the doc may
go to the top level, and places it with `place(.., Last, ..)` instead of
`max(sort_key)` under the database collation. The explicit-key `create`
and `move_to` are unchanged, for the tests that call them.
Routes: create uses `First`, or `After(after_id)`; move uses
`After(after_id)`, else `Before(before_id)`, else `Last`; from-template
uses `Last`. `BadAnchor` is 422 `doc.bad_anchor`. No production code calls
`sort_key_between` any more; `lexorank::between` is called only by
`placement`.
Tests: crates/knot-storage/tests/placement.rs (new) and
crates/knot-server/tests/placement_integration.rs (added to).
Existing test changed (P6): docs_integration.rs
`move_with_unknown_anchor_falls_to_end` encoded the bug that an anchor
outside the group silently moved the page to the end. It now asserts 422
`doc.bad_anchor` and is renamed `move_with_unknown_anchor_is_422`.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Share one copy of the create and move transactions between the explicit-key and the placed paths (controller ruling, amends P8): commit_create (INSERT ... RETURNING, map_unique, audit doc.create, invalidation create, commit), commit_move (UPDATE ... RETURNING, map_unique, NotFound, audit doc.move, invalidation tree-move, commit), reject_cycle (the recursive cycle check), and lock_parent, the parent row read parameterised by lock mode (FOR SHARE for lock_live_parent, FOR NO KEY UPDATE for placement's lock_group). create and move_to keep their steps and order; their existing tests are unchanged and green. - Restore's final UPDATE maps a unique violation to Conflict, as create and move do. A writer that skips the group lock (an old pod during a rolling upgrade) could take the key between restore's read and its write; that was a database error. New test restore_racing_an_unlocked_writer_for_its_key_is_a_conflict. - New test restore_next_to_an_invalid_last_key_goes_last: restore to the top level next to a seeded "z0" last key respreads and places the page last with a valid key. On 70b5412 it panics in sort_key_between. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Workspace import created each page with the sort_key from the export's manifest. Importing the same export twice under a parent collided on those keys: the second copy's pages were skipped and their children grafted onto the import target. Since top-level keys became unique, the same happened at the top level. Import now creates every page with create_placed(.., Placement::Last) under its new parent or the import target. The manifest key only orders siblings: pages are created by depth, then by sort_key byte by byte (COLLATE "C", the sidebar's order), then by manifest index. No existing test changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Known item C5: "Duplicate…" on a page without subpages copies it at once, with nothing on screen while the copy is made and no guard: picking it again made a second copy (RED on HEAD: two requests). - While a page's copy is being made, its row shows "Duplicating…" and its "Duplicate…" menu item is off. Both follow the pending duplicate mutations in the cache (useMutationState), so they hold for a copy asked for in the dialog too, once that is closed. Other pages can still be copied. - The tree's live region says "Duplicating “<title>”…" when the copy starts without a dialog, so a screen reader hears that it is under way. Test: the row says Duplicating…, the live region says so, the item is off and a click on it sends nothing, another page's item stays on, and once the copy is made the status goes and one request was sent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item C6: when a duplicate failed, the sidebar kept its list. But a copy can be stored even though the request failed: it timed out (408) or its answer was lost. That copy then stayed out of the sidebar until something else refreshed it, and the user, seeing no copy, would ask again and get a second one. A failed duplicate, in the dialog or not, now invalidates ["docs"], so a copy stored anyway shows at once. Test: with a server that stores the copy and then answers 408, or never answers, the error is shown, the list is fetched again and the copy's row appears (RED on HEAD: no row). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item C7: since Task F3 the server makes only a few copies at once and answers 503 doc.duplicate_busy (Retry-After: 5) when none comes free within its wait. The sidebar showed the generic "Couldn't duplicate this page." for it, which reads as if the page can't be copied. duplicateErrorText now takes the error (status and code) and says "Too many copies are being made right now. Try again in a moment." for that code, in the dialog or as a toast. A 503 without the code (a proxy in front of the server) keeps the generic message. Tests: both cases in the error table, for the dialog and for a toast (RED on HEAD for the busy code: the generic message). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item D3: duplicate.spec.ts kept its own copy of the CSRF header helper that tree-helpers.ts already has. tree-helpers.ts exports writeHeaders(request) now, and duplicate.spec.ts uses it with page.request (the page's own session). The tiny PNG's comment said 70 bytes; the hex is 69. Both specs are branch-only; no spec that exists on main changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item D2: duplicate_with_subpages took the first of a page's boards and checked only that one existed, so a copy that gained a second board passed. And svgOf compared response bodies without checking the status: two 404 bodies were equal too. boardOf(page, docId) asserts the page has exactly one board and returns it; svgOf(page, board) asserts 200 before returning the SVG. The copy's board must differ from the original's and show the same SVG, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item D1: duplicate_with_subpages duplicated as soon as a fresh browser showed the board's preview SVG, but checked the board's own content (the drawn rectangle) only at the very end. The preview is saved apart from the board's content, so the copy could have been made before the drawing was stored, and the test would fail late, or pass for the wrong reason. The fresh browser that checks the original before the copy now also opens its board and waits until it holds the rectangle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Known item D4: duplicate_with_subpages copied empty subpages, so it only showed that subpages are copied, not that their content is. The Chapter subpage now gets a paragraph and an image (uploaded as its own file, then referenced from Markdown). After the copy, a fresh browser shows the copy's Chapter with that text and an image that loads from a different file. Once the original is deleted for good, the original Chapter's image is gone (404) and the copy's Chapter still shows its own. shownImage() is taken out of expectContentShown() for this, and uploadImage() and childOf() are new helpers. The test.slow() comment counts the Excalidraw loads and browser contexts as they are now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
releasing_on_own_row_does_not_reload (tree-moves.spec.ts, written on
this branch) pressed on B, went 10px down to start the drag and let go
about a px further on B's own row: about 11px of travel in all. Since
the slipped-click rule (a drag that moves nothing after less than one
INDENT of travel is a click), that opens B in the app, so the test's
"the URL stays on A" failed in both repeats of the e2e run. That release
is a click now, which the new a_click_that_slips_sideways_opens_the_page
covers.
The test keeps what it is for, a real drag that ends on its own row with
nothing to drop: it goes off onto A's row first and then back, which is
what its comment already said ("Off and back onto B's own row"). Also
two comment fixes in DocTree.tsx: the mouse drag starts past 6px, and
the browser sends no click at all after a drag (the button goes up over
the drag ghost), rather than one to the ghost.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ec and plan
CHANGELOG [Unreleased]:
- Upgrading: five migrations (v0.8.1..HEAD adds user_last_seen_version,
heal_sort_keys, markdown_renderer_version, blob_copy_pending and
markdown_sweep_attempts); on S3 a purge the bucket refuses now keeps the
page and retries, so the credentials need s3:DeleteObject.
- Added: Duplicate's 503 doc.duplicate_busy with Retry-After.
- Fixed: the first entry says a list item's line break broke export only
after a restore, a template or an import, and that Markdown import made
the damaged shape too; new entries for parent cycles from concurrent
moves and for S3 files served as the bucket's error message.
- Security: pages nested thousands of levels deep crashed the server on
export (pre-existing); nesting is now capped.
knot-api skill: an unknown or foreign {id} on /api/docs/{id} routes is
403 acl.no_grant (the doc-role middleware), not 404, for Duplicate and for
the moved doc of a move; Duplicate's 503 doc.duplicate_busy.
Helm README: the five migrations, s3:DeleteObject in the S3 section and the
0.8.1 upgrade notes, and what the real-data check now fails on and lists.
Spec: status Approved, the committed mockup, FOR NO KEY UPDATE, the move
lock, one-statement rebalance under the DEFERRABLE constraint, Math.trunc
and the small-drag-as-click rule, Duplicate's durable cleanup and busy
answer, the depth limit and depth_limit class, the general gesture rule in
acceptance 5, removed tests in acceptance 6, historical prototype paths.
Plan: P6 lists migrations_apply.rs, the ws_writer harness, the dropIntent
removal and the tree.test.ts imports; P7's knot_heal_spread_key; P8's move
lock; P11's Math.trunc, click rule and gesture rule; historical Reference
paths.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Self-review of the first Fixed entry: "for any list item with a line break that a Markdown import made" read as if the editor had made it, and "the same way" for images pointed at nothing. Say that restore, templates and imports stored the line break in a broken shape, that a Markdown import did so for any list item with a line break, and that images in list items were lost on those same paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ith 4403
Rooms (Ruling F6, superseding the Err part of Ruling F1 round 3):
- An update that doesn't decode is not applied at all. The room, page or
board, now refuses only its sender (4422) and keeps running. It used to
evict itself, closing every other connection with 1012.
- An update yrs rejects part-way is measured over what the apply
integrated (apply_update_measuring now reports `rejected` instead of
failing). Past the limit it is refused and the room evicts itself, as
before. Within the limit the room does what main does: it neither
persists nor fans out the update, and keeps the sender. yrs retries the
blocks it parked earlier inside the same apply and passes on their
error, so a crafted writer could park a block that fails on an innocent
editor's next edit: the room closed that editor and evicted itself, and
every reload parked the block again, locking the editor out for good.
- A board connection gets the 4422 signal a page connection has, so a
board room refuses senders the way a page room does.
- The metric help texts, logs and registry comments say what is counted:
an eviction now only ever means content past the limit, so
knot_room_evicted_total loses its reason label.
Clocks: update_items adds a block's length to its clock with checked
addition. A clock run past u32::MAX panicked in a debug build and wrapped
in a release build; it is now a decode error, read before yrs reads the
update (yrs adds the same clocks unchecked).
Purge: archive's purge queued a revoke for the room and then evicted it at
once, so the room's end closed its sockets with 1012 and clients kept
reconnecting to a purged page. Rooms::revoke_and_evict cancels the room's
new revoked token first; each connection's revoked token is its child, so
every socket closes with 4403. The revoke_all_for_doc doc no longer claims
the shim detects a closed channel.
The crafted over-the-limit update moves from nesting.rs into
knot_test_support::crafted, so the server's socket tests reuse it to
trigger an eviction now that a decode error no longer does.
Existing test changed: crates/knot-crdt/tests/board_fanout.rs builds its
board ConnHandle with ConnHandle::new(tx) instead of ConnHandle { tx }, a
mechanical API adaptation (the struct gained the refused token); no
assertion changed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- duplicate::check runs the checks that need no permit (the user's roles, P15, and a live, non-template page of their workspace), and the route runs it before it waits for one. While both permits were taken, a viewer and a template source got 503 doc.duplicate_busy instead of 403 acl.editor_required and 422 doc.is_template. run() still checks the same, through the same function. - The 503's Retry-After comes from BUSY_WAIT (5 s), the one constant for the wait, instead of a literal "5". - remap: the held items a text's links need are read once per call through a OnceCell instead of an Option<Option<_>>. A doc note says a page with reference links is decoded twice (references, then remap_refs), which is fine within Duplicate's 200-page cap. - Regression guard (F3): in a document counting UTF-16 units, "Grüße " then an embed before a link still remaps the link on its own characters. The UTF-16 tests share two small helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- placement.rs's tests assert that knot_test_support::locks::TOP_LEVEL_LOCK, the copy of the key the deadlock helpers take, equals placement's own TOP_LEVEL_LOCK. A drift would make those helpers take a different lock, and the deadlock tests would pass without testing anything. - reject_cycle's doc comment is reflowed to the surrounding width. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Collision is pointer-first, so a click pressed at a row's top edge that slipped 8px up landed in the row above's middle band and nested the page there. On main that did nothing. Extending ruling web-1 (Ruling F6): until the pointer has travelled one INDENT from the press (the whole path, Euclidean) there is no drop, so no drop line shows and the release moves nothing; a plain press on the row's link opens the page, as the click rule already did for a sideways slip. - That click replaces the history entry when the page is already open, as react-router's Link does, instead of pushing a duplicate. - Duplicate follows one per-page rule on both paths: the dialog's pending state and its guard look only at its own page, so while another page's copy is made the dialog opens ready and works. When that other copy is made while the dialog is open, the copy shows in the list and the app stays in the dialog: it no longer navigates away under it. - The "Duplicating “X”…" announcement is cleared when the copy settles, so copying the same page again is announced again. - insertIndex puts the page last when its anchor isn't among the siblings, as applyOptimisticMove does, and says so. It gave 0 or -1. - Drags of one level in the tests go INDENT + 2: exactly one INDENT sits on the Math.trunc boundary. e2e/flows/tree-reorder.spec.ts, which exists on main, changes only that distance, as Ruling (F6, amends P6) allows; tree-moves.spec.ts and DocTree.dnd.test.tsx are this branch's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cargo.lock only. With 0.13.5 the compat harness (KNOT_COMPAT_REPORT=1) prints the same eleven report lines as F5's run on 0.13.4, the knot-markdown tests and fixtures pass unchanged, and cargo deny's advisories pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CHANGELOG: - Security, the nesting entry: Markdown import and export follow 128 levels and flatten deeper content with no text lost; rooms refuse an editor update nesting past 512 nested types (4422); a single crafted update whose own apply overflows, and deleting a page already stored deeper, remain known limitations as before this release, pointing to the PR. - Security, new: pulldown-cmark 0.13.5 fixes RUSTSEC-2026-0414 (knot never enables wiki links, so it was not affected). - The thanks to @szEvEz names what #42 found: the list export, page order and sidebar drag and drop bugs, and the ask for Duplicate. - The first Fixed entry: a Markdown import broke line breaks, and dropped images, only in items of a tight list (spec A.1), not in any item. The F6 fixes of branch-only regressions (room refusals, purge close code, the vertical slip) get no entries. Spec: the click rule reads "a drop with less than one INDENT of pointer travel moves nothing"; a "Room refusals" rule; section C's projectDrop signature is P11's; "Prototypes and probes" says the probe cases went in as probes, not fixtures (P21). Plan: P11 and Task 13's onDragEnd line carry the F6 click rule, and a "Room refusals" note after P21 records the rule added in F1 and amended in F6. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…is made Self-review of the F6 per-page Duplicate rule: the two-pages test pressed Duplicate in the second page's dialog only after the first copy had been made, so a guard that still looked at every page's copy passed it. The dialog is now confirmed while the first copy is on its way, and both answers are held until the test releases them: the first is made under the open dialog (no navigation, the dialog stays), then the second closes it and opens its copy. Both mutants (a global guard in confirmDuplicate, or `pending` for any copy) now fail the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ruling (F6 review, fix round): (a) Pin the update reader to yrs. A differential test compares knot_crdt::update_items with yrs's Update::decode_v1 on whether each update decodes and, when both decode it, on the items it holds: knot-crdt's test updates (update_items.rs, nesting.rs, the engine's and the conformance tests', the crafted base and attack) and every prefix of each (nesting.rs's update 100,000 types deep only whole); the compat corpus (generated documents in both shapes, the 50 random seeds, every fixture and probe through both importers), each document's state, a diff from half its clocks, a deletion and the state after it, and every prefix of the files' updates. It found the reader accepting updates that end inside the delete set, which yrs refuses (47 + 12 + about 4,100 prefixes; none the other way, none on items). The reader now reads past the delete set as IdSet::decode does. (b) Split the sidebar's slip rule. A drop moves nothing while the pointer is less than one INDENT from the press (straight distance), so a jittery 8px slip up no longer nests the page; it opens the page only when the whole path is under one INDENT too, so a drag that goes out and comes back never navigates. Spec and plan wording follow. (c) One BoardStore double in tests/nesting.rs: StoredBoard replaces CapturingBoard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- tree: judge a drop near its press on dnd-kit's scroll-adjusted delta, the one the drop line is drawn from, in track() and onDragEnd alike. The release point was measured on screen, so after the sidebar scrolled during a drag a drop line could show and the drop then move nothing, where main moved the page. The path-length click rule is unchanged. New dnd test scrolls the sidebar mid-drag; the dnd tests' tree now sits in a scrolling sidebar, as AppShell's does. - crdt/markdown tests: one copy of the reader-vs-yrs differential (rule and failure message) in knot-crdt's tests/common/mod.rs; knot-markdown's update_reader test includes it by #[path]. - crdt: the update reader's module doc lists the delete-set client difference (yrs debug-asserts the 53-bit limit; the reader reads the raw u64). - spec/plan: the near rule names the frame it is measured in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ConnHandle::refused says nothing more a refused connection sends is applied. It didn't hold: a page room refused a re-join only when the connection was revoked, so a SyncStep1 queued behind the refused update registered the connection again and its pipelined updates applied; a board room applied a refused sender's queued updates anyway, since it takes updates from any connection; and both fanned out its presence. Both rooms now ignore everything a connection sends once its refused token is cancelled, using that token and no new state: - page room: on_join refuses a handle whose refused token is cancelled, as it does a revoked one; presence is shared only from connections the room holds, the rule on_inbound already applies to updates; - board room: on_join refuses such a handle; refuse_sender keeps the handle (token cancelled) until the connection leaves, so on_inbound and presence can tell it was refused (unjoined senders still work, as board_fanout.rs needs), and every fan-out skips it, so a closed channel never drops it early. refuse_sender's doc is corrected. Tests (crates/knot-crdt/tests/nesting.rs), RED on HEAD for both rooms: after a prefilter or decode refusal, a queued SyncStep1 doesn't re-register the connection, and its queued edits and presence, before and after a peer's edit is fanned out, are not applied, stored or fanned out. The branch-only in-module test board_room_refuses_a_deeply_nested_update (added in 11c1f49, not on main) sent its follow-up update from the refused sender, encoding the old behaviour; it now sends it from another connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plan P6 now lists, before "Nothing else changes", the existing tests the branch changed that it didn't name yet: - documents.rs purge_takes_rows_stamped_with_the_root_outside_its_subtree, distinct keys under the duplicate-keys clause (commit 43082af); - board_fanout.rs's join_conn, ConnHandle::new(tx) (Task F6, Ruling (F6 review, amends P6)); - tree-reorder.spec.ts: the un-nest case's one level left (Task 15) and INDENT + 2 for exact-INDENT drags (Task F6, Ruling (F6, amends P6)). CHANGELOG: - Changed: the sidebar's keyboard drag is gone; Alt with the arrow keys and the page menu's Move up, Move down, Indent and Outdent move pages. dnd-kit's keyboard sensor started and dropped on Space or Enter, so the entry names both. - Upgrading: the order-key repair locks `documents` until its migration commits (its first statement is ALTER TABLE, which takes an ACCESS EXCLUSIVE lock, and sqlx runs each migration in a transaction), so requests that touch pages wait during the pre-upgrade migrate job; and rolling back to 0.8.1 needs a database restore, because 0.8.1 runs its embedded migrator on start, which refuses a database holding migrations it doesn't know (sqlx VersionMissing). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y the board refusal docs The page room persists asynchronously, so its catch-up tick can fan an earlier sentinel out again before the next one. a_refused_connection_stays_refused now allows exactly that and nothing else ahead of each sentinel, as fanned_out's doc describes, instead of asserting nothing came first. The board room can only cancel the refusal token of a connection it holds; the docs on ConnHandle::refused and refuse_sender now say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 28, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A refused update stays in the client's doc, so a reconnect handed it over again in the answer to the server's SYNC_STEP_1 and was refused again, about every half second (the backoff resets on each successful open). When the post-apply check refused it, every reconnect evicted the room and bounced every editor of the page. KnotProvider and BoardProvider now treat 4422 like 4403: a "refused" status and no reconnect. The status chip reads "Changes refused" with a hint to reload, which starts over from the server's state. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 28, 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
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.
Supersedes #42 by @szEvEz. #42 found the list export, page order and sidebar drag-and-drop bugs, asked for Duplicate and proposed the first fixes. This branch rebuilds the fixes from scratch, and goes further where the root cause turned out to be wider. Thank you, @szEvEz.
Summary
The guarantee for existing pages: a healthy page (one whose current export is a fixed point) exports byte for byte as before, unless it falls in a named bug class; no export loses text. Two harnesses check this: one over a corpus in CI, one opt-in against a real database.
How the sidebar decides a drop:
How a room (page or board) treats an inbound update:
Duplicate,
POST /api/docs/{id}/duplicate:{"include_subpages": true});blob_copy_pending);413 doc.duplicate_too_largeover 200 pages or 200 MiB;503 doc.duplicate_busywithRetry-After: 5(two copies at a time per server, five-second wait);403 acl.editor_requiredfor viewers,422 doc.is_template,409 doc.archived;409 doc.duplicate_conflictif the source changes mid-copy, or a file it shows is missing from the store (kept on purpose; to revisit if it bites);Evidence
Compatibility harness:
crates/knot-markdown/tests/{compat,tasks_compat}.rsrun againstknot_markdown::legacy, a frozen copy of main's serializer, importer and task extractor (behind the test-onlylegacy-compatfeature).pulldown-cmark0.13.5.KNOT_COMPAT_REPORT=1run.ordered_checkbox: 998 (checkboxes in numbered lists are now exported);multi_block_item: 49;nested_tasklist: 15;quote_block_sequence: 25;depth_limit: nesting past 128, flattened; the only class that isn't a bug legacy had.Real-data check: opt-in and read-only (
default_transaction_read_only=on, no migrations). It runs from a source checkout of this commit:KNOT_COMPAT_DATABASE_URL=postgres://… \ cargo test -p knot-server --test markdown_compat -- --ignored --nocaptureIt reports:
/taskslisting would change, by class, with diffs;It fails if:
/taskswould stop listing one task per checkbox;On the final head it passes on the three databases its own tests seed (current schema with a cross-workspace link, deep pages, a database from before the upgrade). No database with real pages has been checked yet: see the manual checks below.
Regression tests: each behaviour fix comes with tests that the task reports show failing on the branch before the fix. Some rows:
409 doc.sort_key_conflict("Couldn't create document"); that level is blocked>deep, then replace itSuites on the final head:
cargo nextest run --workspace --all-features)pnpm test)ws-reconnecttests need toxiproxy and fail identically on maincargo fmt,clippy -D warnings,pnpm tsc,pnpm lintcargo deny checkpulldown-cmarkto 0.13.5)Existing tests changed (each mechanical or encoding a bug; the plan's P6 and the rulings list them; none checks less)
lexorank.rsin-module tests:.unwrap()onbetween.move_with_unknown_anchor_falls_to_end→move_with_unknown_anchor_is_422(422 doc.bad_anchor).documents.rs,comments_integration.rs,shares_integration.rs, andsearch_integration.rs'smake_doc.e2e/flows/tree-reorder.spec.tsis rewritten:Math.truncrounding.e2e/flows/tree-dnd.spec.tsis removed: it checked only "no error toast".sidebar-resize.spec.ts: selectors follow the row markup (role="button"misuse removed).migrations_apply.rs: the table list gains the two new tables.ws_writer.rsandboard_fanout.rsharnesses: connections carry the new refusal tokens.tree.test.ts:dropIntentcases are removed withdropIntent;descendantIds/moveArgstests run against a test-only copy,tree.legacy.ts.ContextMenu.test.tsxandDocTree.archive.test.tsx.Behaviour changes to know about
Drop depth. A before/after drop next to a row at another depth keeps the dragged page's depth as far as the gap allows. Moving the pointer sideways changes it by one level per INDENT, and the drop line shows the result. Main took the hovered row's depth.
Small drags are clicks. A release less than one INDENT from the press moves nothing. If the pointer barely moved and it was a plain press on the link, the page opens.
Keyboard and touch.
Orphan rows. A template's child, or a live child of an archived page, is drawn at the top level. Moving it there by menu, keys or drag makes it a real top-level page.
API.
422 doc.bad_anchor(was: put last).409 doc.restore_conflict(was 500).409 doc.sort_key_conflict.parent_idthat is unknown or in another workspace →404 doc.not_foundon create, move, from-template and import (import used to answer 403; the others used to accept another workspace's parent).Workspace import ignores the manifest's keys. Pages get fresh keys in the export's order, after the target's existing pages.
My Tasks lists these now:
A bullet that merely starts with
[ ]is no longer listed.Depth cap.
Live connections. When a room ends for any reason, including a panic, every socket now closes with 1012, so the client reconnects. A refused sender gets 4422, and nothing more it sends (edits, presence, a re-join) is applied. The page and board clients treat 4422 as final: they stop reconnecting and show "Changes refused" until a reload, since a reconnect would hand the refused update over again. A page room now shares presence only from connections it still holds, so a revoked or slow-consumer-dropped connection no longer shows a cursor.
S3.
s3:DeleteObject. A refused delete keeps the page and the hourly job retries, instead of deleting the page and orphaning its files.Metrics (new):
knot_room_updates_refused_totalknot_room_evicted_totalknot_markdown_unknown_nodes_totalknot_markdown_depth_limited_totalMerge Danger
Door: one-way for data, two-way for the UI.
Four new migrations:
heal_sort_keys:NULLS NOT DISTINCT, deferrable);documentsuntil it commits, so page requests wait during the pre-upgrade job on a large database.markdown_renderer_version: marks every cached render stale. A background sweep then re-renders every live page, about ten pages every two seconds on one replica (at most about 18,000 an hour). Until a page is reached, search, share links, zip export and My Tasks show its old render.blob_copy_pendingandmarkdown_sweep_attempts: bookkeeping tables.Rolling back to 0.8.1 needs a database restore. Run the real-data check on a backup first. The gesture changes are two-way doors: easy to tune once they've been used.
Blast Radius: wide.
Every page's export path, My Tasks, page create/move/restore/import, the sidebar, live editing rooms, and S3 storage. During a rolling upgrade, old pods can answer
409 doc.sort_key_conflicton create or move until they're replaced, and new pods can too when an old pod's unlocked write takes a key. Don't run workspace imports mid-rollout.Two notes for whoever merges:
Known limitations
Tracked in #53; other follow-ups in #54. All of these are pre-existing on main unless marked new.
ApplyUpdate,ReplaceWithMarkdown) and bus replay keep a partly applied live doc on an apply error.Follow-ups
documents_tree_idxcleanup.[vite] ws proxy errornoise.knot-test-support.Manual checks before merge
s3:DeleteObject.🤖 Generated with Claude Code