Skip to content

feat: import a Markdown file as a page - #9

Merged
christianhuening merged 13 commits into
mainfrom
feat/import-markdown-page
Sep 4, 2026
Merged

feat: import a Markdown file as a page#9
christianhuening merged 13 commits into
mainfrom
feat/import-markdown-page

Conversation

@christianhuening

Copy link
Copy Markdown
Contributor

Closes #8.

Both things a user naturally tries to get a Markdown document into knot were dead ends: there was no upload control, and pasting the source landed as literal text. The backend endpoint (POST /api/docs/{id}/markdown) already existed and nothing in web/src had ever called it.

The issue's open question

It asked whether importing into a non-empty page duplicates it. It does, and there's now a test that pins it:

expected: replaced
actual:   "# Original\n\n# Imported\n"

import_inline hands its parsed bytes to Event::ApplyUpdate, which Yjs merges into the live fragment. That's correct for the only existing caller (from_template always targets a fresh doc) and wrong for importing over a page you've already used.

Rather than restrict imports to empty pages, this adds ?mode=replace|append, routing replace through the Event::ReplaceWithMarkdown that history restore already uses. append stays the wire default, so from_template and any existing API client are byte-for-byte unaffected; only the new UI opts into replace.

What's here

Import control — a FileUp button beside Export, editor+ only. Guards the server's 1 MB cap client-side (axum's own rejection is a bare 400), checks whether the page has content, confirms before overwriting, then POSTs. No refetch needed: the room actor fans the replace out over the WebSocket the editor is already on.

PastelooksLikeMarkdown converts on a fence, a GFM table or a ## heading; otherwise it needs two distinct cue kinds or a repeated block cue. Headings only count at a block start, which is what stops a pasted Python or shell snippet reading as a pile of # headings. The asymmetry is deliberate: a miss pastes plain text exactly as before, a false positive would mangle what someone pasted. Three opt-outs: a rich-text clipboard, a code block, and ⌘⇧V.

Converted HTML goes through marked → task-item promotion → a new DOMPurify profile before Tiptap sees it. Markdown passes raw HTML straight through, so that sanitizer is required, not decorative.

Two fixes found along the way

  • Imported checklists rendered as plain bullets. from_markdown stores checked as the Yjs string "true"/"false", but TaskListExtension only matched booleans. Pre-existing — it hit create-from-template too — but import made it reachable. The Markdown round-trip and doc_tasks rows were correct all along; only rendering was wrong.
  • pnpm lint was red on main (4 errors, 1 warning) because CI ran tsc and test but not lint. Two of those were real latent bugs: onClick and onPickTemplate are typed to return void, so their inline async handlers left rejections unhandled — a failed setTemplate would have surfaced as an unhandled rejection instead of the intended error toast. All five fixed at the cause, and pnpm lint is now a CI step.

Testing

  • Rust — new markdown_import_integration.rs: append-duplicates (pins the default), replace-swaps, replace-into-empty, bad mode → 400, viewer → 403, anon → 401, non-UTF-8 → 422, and attribution on the replace path.
  • Vitest — 20 cases over the paste heuristic including the false-positive ones (C# is fine, interleaved Python comments, a #-commented config), the sanitizer, the import button's confirm gate, and the checklist normaliser.
  • Playwright — import into an empty page, replace a non-empty one, paste Markdown source, an imported checklist keeping its checkboxes, plus a templates spec covering the two handlers the lint fix refactored (neither had any coverage).

331 Rust tests, 103 vitest, clippy, fmt and tsc all green. E2E is 42 passing; the 4 failures (oidc, ws-reconnect, editor-toolbar, one tree-reorder) reproduce identically on main with this branch stashed — two of them are macOS-only, since the toxiproxy sidecar uses network_mode: host and lands in Docker's Linux VM rather than the host.

Not doing

Titles are never derived from a leading #; the workspace zip import is untouched; relative image paths in an imported file are imported as-is and won't resolve.

Design and plan are committed under docs/superpowers/.

🤖 Generated with Claude Code

christianhuening and others added 13 commits September 4, 2026 16:19
Covers issue #8: an "Import Markdown…" control on the doc page plus
Markdown-aware paste in the editor. Resolves the issue's open question by
adding ?mode=replace|append to the existing POST /api/docs/{id}/markdown,
defaulting the wire to today's append so from-template is unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eight TDD tasks from the ?mode= backend switch through the import
control, the paste heuristic, and e2e coverage.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The variant hardcoded by_user_id: None, so a full-document replace was
the one mutation that landed in doc_updates unattributed. Existing
callers pass None, so their behaviour is unchanged; the markdown import
endpoint will pass the importing user.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The endpoint only ever merged, because its one caller (from-template)
always targets an empty doc. Importing over a page with content
duplicated it — the new integration test pins that as "# Original\n\n#
Imported\n". Replace routes through ReplaceWithMarkdown; append stays the
wire default so existing callers are untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Markdown passes raw HTML through, so anything converted client-side must
be sanitized before Tiptap parses it. Returns a fragment so the task-list
rewrite can post-process without reparsing unsanitized markup — which
would be enough to start an <img> load and fire its onerror.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pure module so the heuristic — the risky half of Markdown paste — is
testable without an editor. Errs toward not firing: a miss pastes plain
text as before, a false positive would mangle the paste. Headings only
count at a block start, which is what keeps a pasted Python or shell
snippet from reading as a pile of # headings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pasted Markdown landed as literal text. handlePaste now converts it when
it unambiguously looks like Markdown, with three opt-outs: a rich-text
clipboard, a code block, and ⌘⇧V.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reads a local .md file and replaces the page body via
POST /api/docs/{id}/markdown?mode=replace, confirming first when the page
already has content. Its own component so the confirm gate is testable
without mounting DocPage.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the header control next to Export, plus e2e coverage for importing
into an empty page, replacing a non-empty one, and pasting Markdown
source.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A list_item's `checked` attribute arrives as a boolean when you type
"[ ] " in the editor, but as the string "true"/"false" when the document
was parsed by knot_markdown::from_markdown — Yjs XML attributes are
strings, and y-prosemirror hands whatever is stored straight through as
the node attribute. renderHTML only matched the boolean form, so
anything built from Markdown server-side lost its checkboxes: imported
files, and pages created from a template.

Surfaced by the new Markdown import: an imported checklist rendered as
plain bullets while its markdown round-trip and doc_tasks rows were
correct all along.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`pnpm lint` has been failing on main with 4 errors and 1 warning. CI only
ran `tsc --noEmit` and `pnpm test`, so nothing caught it. Each fix removes
the cause rather than suppressing the rule:

- DateTimeExtension: drop `as Partial<RawCommands>` and the import it
  needed; the inferred type already satisfies addCommands.
- DateTimeExtension.test: `editor.getJSON()` is already assignable to
  Record<string, unknown>, so the double assertion was dead weight.
- tree.ts: drop an eslint-disable for `no-console`, a rule not enabled.
- DocPage / DocTree: `onClick` and `onPickTemplate` are typed to return
  void, so an inline `async` handler left rejections unhandled. Extracted
  to named functions fired with `void`, matching KnotEditor's idiom.

Adds `pnpm lint` to the web CI job so this cannot rot again, and an e2e
spec for the two template handlers the refactor touched — neither had
any coverage.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cargo-deny went red on this PR for reasons that have nothing to do with
it — no Rust dependency changed here. Both are transitive:

- h2 0.4.15 -> 0.4.19. RUSTSEC-2026-0258: h2 accepted and queued empty
  DATA frames without limit, so a stream that was not actively drained
  could grow memory unboundedly, or panic on length overflow. Low
  severity, reached via hyper under axum and reqwest. Patched in 0.4.16.
- chacha20 0.10.1 -> 0.10.2. 0.10.1 was yanked, and deny.toml sets
  yanked = "deny", so the next run would have failed on it regardless.
  Reached via rand.

The advisory postdates main's last green run (2026-08-18), so main is
red on this too and this fixes it there as well.

Also re-points four crates from windows-sys 0.61.2 to 0.52.0 and one from
windows-link 0.2.1 to 0.1.3. That is cargo re-resolving duplicates during
the update, not a deliberate pin — both versions were already in the lock,
and these are cfg(windows) deps that no target we build touches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@christianhuening
christianhuening merged commit e0d19a7 into main Sep 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: import a Markdown file as a page

1 participant