Compare commits
23 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| d89650a45e | |||
| b1e48d3765 | |||
| 293348f9dc | |||
| 330837cfa6 | |||
| 916b24e3ff | |||
| ecf022ffca | |||
| 62af116271 | |||
| e9d5d493d3 | |||
| db9ed51e01 | |||
| 963822bd28 | |||
| c452902432 | |||
| 895173b176 | |||
| 45d5ae1601 | |||
| ec30e6c08a | |||
| db9f29c16b | |||
| 82411f8707 | |||
| af481d401a | |||
| b349676eae | |||
| 438ef091f9 | |||
| 768d135a19 | |||
| c90caeb21a | |||
| 5664da57ad | |||
| c39fab70c1 |
@@ -72,7 +72,10 @@ git log -1 --format='Author: %an <%ae>%nCommitter: %cn <%ce>'
|
|||||||
|
|
||||||
### 4. Push and PR to develop
|
### 4. Push and PR to develop
|
||||||
|
|
||||||
PRs always target `develop`. The `claude_code` password lives in the macOS
|
PRs always target `develop`. Two different mechanisms are involved: **pushing
|
||||||
|
commits is git-native** (the Gitea MCP cannot push local git history, so the
|
||||||
|
branch is still pushed with `git push`), while **the PR itself is opened through
|
||||||
|
the Gitea MCP** (see below). The `claude_code` password lives in the macOS
|
||||||
keychain as a **generic password** under service `gitea-claude-code` (do not
|
keychain as a **generic password** under service `gitea-claude-code` (do not
|
||||||
duplicate it as an internet-password for `gitea.vvzvlad.xyz` — that creates a
|
duplicate it as an internet-password for `gitea.vvzvlad.xyz` — that creates a
|
||||||
conflict with the owner's account in the git credential helper):
|
conflict with the owner's account in the git credential helper):
|
||||||
@@ -94,18 +97,24 @@ git remote set-url gitea "$ORIG_URL"
|
|||||||
unset AGENT_PASS SAFE_PASS
|
unset AGENT_PASS SAFE_PASS
|
||||||
```
|
```
|
||||||
|
|
||||||
The PR is created via the Gitea REST API (Basic Auth as `claude_code`):
|
The PR is opened through the **Gitea MCP** (server `gitea`), not `curl`/`tea` —
|
||||||
|
the MCP authenticates in-process, so no keychain lookup or Basic-Auth is needed.
|
||||||
|
Call `pull_request_write` with:
|
||||||
|
|
||||||
```bash
|
- `method: "create"`
|
||||||
curl -s -X POST \
|
- `owner: "vvzvlad"`, `repo: "gitmost"`
|
||||||
-u "claude_code:$(security find-generic-password -s gitea-claude-code -w)" \
|
- `base: "develop"`, `head: "<branch>"`
|
||||||
-H "Content-Type: application/json" \
|
- `title`, `body` — in the body: what was done, what is out of scope,
|
||||||
-d @pr_body.json \
|
verification results (tsc/lint/tests).
|
||||||
"https://gitea.vvzvlad.xyz/api/v1/repos/vvzvlad/gitmost/pulls"
|
|
||||||
```
|
|
||||||
|
|
||||||
`base: develop`, `head: <branch>`. In the PR body: what was done, what is out
|
Manage and read PRs through the same server: `list_pull_requests`,
|
||||||
of scope, verification results (tsc/lint/tests).
|
`pull_request_read` (`get`, `get_diff`, `get_files`, `get_status`),
|
||||||
|
`pull_request_review_write`.
|
||||||
|
|
||||||
|
**Identity note:** the MCP acts under its **own** configured Gitea token (verify
|
||||||
|
with `get_me`), a different account from the `claude_code` used for git
|
||||||
|
commits/pushes in §3. Only the forge API calls (PR / issue / review) go through
|
||||||
|
the MCP account; the commits themselves stay authored as `claude_code`.
|
||||||
|
|
||||||
> If push fails with `User permission denied for writing`, then `claude_code`
|
> If push fails with `User permission denied for writing`, then `claude_code`
|
||||||
> lacks collaborator rights on the repo. Ask the owner to add them (once, via
|
> lacks collaborator rights on the repo. Ask the owner to add them (once, via
|
||||||
@@ -152,23 +161,25 @@ below.
|
|||||||
| Agent user (Gitea/git) | `claude_code` |
|
| Agent user (Gitea/git) | `claude_code` |
|
||||||
| Agent email | `claude_code@vvzvlad.xyz` |
|
| Agent email | `claude_code@vvzvlad.xyz` |
|
||||||
| Keychain password | `security find-generic-password -s gitea-claude-code -w` |
|
| Keychain password | `security find-generic-password -s gitea-claude-code -w` |
|
||||||
| PR API | `https://gitea.vvzvlad.xyz/api/v1/repos/vvzvlad/gitmost/pulls` (here `gitmost` is the repo's real slug on the server) |
|
| Forge API (PR / issue / review / reads) | **Gitea MCP** — server `gitea` (`pull_request_write`, `issue_write`, `list_pull_requests`, `pull_request_read`, `label_read`, …). Authenticated in-process; acts under its own token — check with `get_me`. Repo slug on the server is `gitmost`. |
|
||||||
| Base branch | `develop` |
|
| Base branch | `develop` |
|
||||||
| `origin` | GitHub mirror `vvzvlad/gitmost` — **do not push**, updated by the owner's CI |
|
| `origin` | GitHub mirror `vvzvlad/gitmost` — **do not push**, updated by the owner's CI |
|
||||||
| `upstream` | The original Docmost — **never push** |
|
| `upstream` | The original Docmost — **never push** |
|
||||||
|
|
||||||
## Creating issues (Gitea `tea` CLI)
|
## Creating issues (Gitea MCP)
|
||||||
|
|
||||||
Issues are filed with the official Gitea CLI `tea`, already logged in as
|
File issues through the **Gitea MCP** (server `gitea`), not a CLI — call
|
||||||
`claude_code` (`tea logins list` shows the `gitea` login as default):
|
`issue_write` with:
|
||||||
|
|
||||||
```bash
|
- `method: "create"`
|
||||||
tea issues create --repo vvzvlad/gitmost --labels feature \
|
- `owner: "vvzvlad"`, `repo: "gitmost"`
|
||||||
--title '<title>' --description "$(cat body.md)"
|
- `title`, `body`
|
||||||
```
|
- `labels` — an array of label **IDs** (numbers), *not* names. Resolve a name
|
||||||
|
such as `feature` to its id first with `label_read` (`method: "list"`), then
|
||||||
|
pass e.g. `labels: [<id>]`.
|
||||||
|
|
||||||
> Gotcha (tea 0.14.1): the issue body flag is `--description`/`-d`, **not**
|
Read issues with `list_issues`, `issue_read`, or `search_issues`. The MCP is
|
||||||
> `--body` — passing `--body` fails with `flag provided but not defined: -body`.
|
authenticated in-process, so no `tea`/`curl` and no keychain lookup are needed.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
+70
-2
@@ -14,8 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
- **Place several images side by side in a row.** A new "Inline (side by
|
- **Place several images side by side in a row.** A new "Inline (side by
|
||||||
side)" alignment mode in the image bubble menu renders consecutive inline
|
side)" alignment mode in the image bubble menu renders consecutive inline
|
||||||
images as a row that wraps onto the next line on narrow screens. Unlike the
|
images as a row that wraps onto the next line on narrow screens. The row is
|
||||||
float modes, text does not wrap around inline images. The mode round-trips
|
centered horizontally by default in modern browsers (CSS `:has()`), falling
|
||||||
|
back to start-aligned rows in browsers without support. Unlike the float
|
||||||
|
modes, text does not wrap around inline images. The mode round-trips
|
||||||
losslessly through markdown as `data-align`, like the other alignment
|
losslessly through markdown as `data-align`, like the other alignment
|
||||||
values.
|
values.
|
||||||
|
|
||||||
@@ -84,6 +86,53 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
with the `||text||` input rule; the rendered span blurs until clicked to reveal.
|
with the `||text||` input rule; the rendered span blurs until clicked to reveal.
|
||||||
The mark is preserved losslessly through Markdown export/import (as a raw
|
The mark is preserved losslessly through Markdown export/import (as a raw
|
||||||
`<span data-spoiler="true">…</span>`) and on public shares. (#259)
|
`<span data-spoiler="true">…</span>`) and on public shares. (#259)
|
||||||
|
- **Dock the AI chat window into the side menu.** The floating chat window can
|
||||||
|
be pinned to the sidebar — drag it onto the navbar (a drop-zone highlight
|
||||||
|
shows where it lands) or use the new "Dock to sidebar" header button; while
|
||||||
|
docked it fills the sidebar area and follows its live size. "Undock" (or
|
||||||
|
dragging it back out) restores the floating window, a collapsed/absent
|
||||||
|
sidebar falls back to floating, and the docked state survives a reload.
|
||||||
|
(#276, #282)
|
||||||
|
- **Hovering commented text shows the comment thread in a tooltip.** Pointing
|
||||||
|
at a highlighted comment mark pops a small card with the author and plain
|
||||||
|
text of the root comment and its replies, so a thread can be skimmed without
|
||||||
|
opening the side panel. The card appears after a short delay (no flicker on a
|
||||||
|
passing glance), skips resolved and text-less threads, and dismisses on
|
||||||
|
scroll or click — clicking a mark still opens the comments panel. (#268,
|
||||||
|
#271)
|
||||||
|
- **"Move to trash" button in the temporary-note banner.** Besides "Make
|
||||||
|
permanent", the banner on an open temporary note now also offers to trash the
|
||||||
|
note immediately instead of waiting out its lifetime. It reuses the regular
|
||||||
|
soft-delete path, so the "Page moved to trash" undo toast is the safety net —
|
||||||
|
no confirmation dialog. (#273, #277)
|
||||||
|
- **Code-block controls float as an overlay instead of taking a row above the
|
||||||
|
code.** The language selector and copy button now sit in the block's top-right
|
||||||
|
corner, and the selector stays invisible until the block is hovered or the
|
||||||
|
selector is focused, so reading code is chrome-free. In read-only views only
|
||||||
|
the copy button renders. (#275, #278)
|
||||||
|
- **The AI agent is told about your page edits between turns.** The server
|
||||||
|
snapshots the open page's Markdown at the end of every agent turn and, on the
|
||||||
|
next turn, injects a unified diff of what changed in between, so the agent
|
||||||
|
knows its earlier copy of the page is stale and builds on the user's edits
|
||||||
|
instead of reverting or overwriting them. The diff is whitespace-normalized
|
||||||
|
(pure formatting churn injects nothing) and size-capped, with a hint to
|
||||||
|
re-read the full page via `getPage` when truncated. (#274, #281)
|
||||||
|
- **Stress-accent button (U+0301) in the bubble menu.** Select a vowel and
|
||||||
|
toggle a combining acute accent over it — a Russian-style stress mark. The
|
||||||
|
accent is stored as plain text (no custom mark), so it survives Markdown/HTML
|
||||||
|
export, full-text search and public shares unchanged; the toggle is a single
|
||||||
|
undo step and re-clicking removes the accent. (#270, #280)
|
||||||
|
- **Reading position survives a reload.** The editor remembers how far you
|
||||||
|
scrolled in each page (per tab, in `sessionStorage`) and restores that
|
||||||
|
position after an F5 or reopening the document, waiting for the collaborative
|
||||||
|
content to finish laying out first. A URL `#hash` anchor still wins — restore
|
||||||
|
is a no-op then. (#266, #267)
|
||||||
|
- **The slash menu finds commands typed in the wrong keyboard layout.** A query
|
||||||
|
typed with the wrong layout active (e.g. `/сщву` for `/code`, or `/cyjcrf`
|
||||||
|
for the Cyrillic «сноска» → Footnote) is additionally remapped ЙЦУКЕН↔QWERTY
|
||||||
|
by physical key position and matched against the commands; genuine Cyrillic
|
||||||
|
search terms keep priority over remapped candidates, and short wrong-layout
|
||||||
|
prefixes match by command title. (#283, #285, #287)
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
||||||
@@ -149,6 +198,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
emits a single-use "intentional clear" signal that lets exactly that one empty
|
emits a single-use "intentional clear" signal that lets exactly that one empty
|
||||||
write through the guard, so genuinely emptying a page is persisted while
|
write through the guard, so genuinely emptying a page is persisted while
|
||||||
accidental empties are blocked. (#248, #251)
|
accidental empties are blocked. (#248, #251)
|
||||||
|
- **Ctrl+Z works again right after using a table menu.** Closing a table
|
||||||
|
row/column menu (grip or chevron) left focus on the menu's portaled target
|
||||||
|
outside the editor, so undo keystrokes went nowhere until you clicked back
|
||||||
|
into a cell. The editor is now refocused after the menu closes — unless you
|
||||||
|
deliberately moved focus to another input or editable (e.g. the page title).
|
||||||
|
(#269, #279)
|
||||||
|
- **The AI reindex progress counter no longer freezes at 0.** Right after
|
||||||
|
"Reindex now" the client could read the stale pre-reindex snapshot of an
|
||||||
|
already-indexed workspace (`reindexing=false`, all pages counted) as
|
||||||
|
"finished" and stop polling on the very first tick, leaving the counter
|
||||||
|
frozen until a manual reload. Polling now keeps going until it has actually
|
||||||
|
observed the active run. (#262, #264)
|
||||||
|
- **An MCP edit can no longer be silently lost to a duplicate collab document.**
|
||||||
|
When the agent addressed a page by its short slugId, the MCP opened a
|
||||||
|
collaboration document named after that slugId while the web editor always
|
||||||
|
uses the page's canonical UUID — two independent live documents for one page,
|
||||||
|
whose debounced stores clobbered each other. The MCP now resolves every page
|
||||||
|
id to the canonical UUID before opening the collab doc (a UUID input
|
||||||
|
short-circuits locally; a slugId is resolved once and cached). (#260, #265)
|
||||||
|
|
||||||
### Security
|
### Security
|
||||||
|
|
||||||
|
|||||||
@@ -104,7 +104,7 @@ community feature, with no enterprise license. Open it from the page header; the
|
|||||||
- ✅ **Page templates** — flag a page as a template and embed its whole content live into other pages; edits to the template propagate to every place it is inserted (whole-page transclusion on top of the existing synced blocks).
|
- ✅ **Page templates** — flag a page as a template and embed its whole content live into other pages; edits to the template propagate to every place it is inserted (whole-page transclusion on top of the existing synced blocks).
|
||||||
- ✅ **Public-share AI assistant** — anonymous visitors of a shared page can ask the AI agent, scoped strictly to that share's page tree (read-only, share-scoped search), behind a workspace toggle.
|
- ✅ **Public-share AI assistant** — anonymous visitors of a shared page can ask the AI agent, scoped strictly to that share's page tree (read-only, share-scoped search), behind a workspace toggle.
|
||||||
- ✅ **Footnotes** — academic-style footnotes: a numbered superscript reference inline (read it in place via a hover popover), with the note text living as a real, editable block at the bottom of the page; auto-numbered, collaboration-safe, and round-trips through Markdown export/import and the AI agent / MCP.
|
- ✅ **Footnotes** — academic-style footnotes: a numbered superscript reference inline (read it in place via a hover popover), with the note text living as a real, editable block at the bottom of the page; auto-numbered, collaboration-safe, and round-trips through Markdown export/import and the AI agent / MCP.
|
||||||
- ✅ **Temporary notes** — mark a note as temporary and it auto-moves to Trash after a configurable per-workspace lifetime (default 24h) unless made permanent first; create one in a click from the Home screen, any space overview, or the space sidebar, with a "Make permanent" rescue banner on the open note.
|
- ✅ **Temporary notes** — create a note as temporary and it auto-moves to Trash after a configurable per-workspace lifetime (default 24h) unless made permanent first; create one in a click from the Home screen, any space overview.
|
||||||
|
|
||||||
### In progress
|
### In progress
|
||||||
|
|
||||||
@@ -187,14 +187,17 @@ start the new migrations apply on top of your existing schema (`CREATE EXTENSION
|
|||||||
- Spaces
|
- Spaces
|
||||||
- Permissions management
|
- Permissions management
|
||||||
- Groups
|
- Groups
|
||||||
- Comments (with resolve / re-open)
|
- Comments (with resolve / re-open and hover tooltips showing the comment text)
|
||||||
- Page history
|
- Page history
|
||||||
- Search
|
- Search
|
||||||
- File attachments
|
- File attachments
|
||||||
- Embeds (Airtable, Loom, Miro and more)
|
- Embeds (Airtable, Loom, Miro and more)
|
||||||
- Translations (10+ languages)
|
- Translations (10+ languages)
|
||||||
- Embedded MCP server (`/mcp`)
|
- Embedded MCP server (`/mcp`)
|
||||||
- AI agent chat over your wiki (read + write, RAG search, external MCP / web access)
|
- AI agent chat over your wiki (read + write, RAG search, external MCP / web access); the chat window docks into the side menu, and the agent is told about your in-page edits between turns
|
||||||
|
- Code-block buttons as an overlay, with the language selector revealed on hover
|
||||||
|
- Stress-accent button (U+0301) in the bubble menu
|
||||||
|
- Reading scroll position restored on reload
|
||||||
|
|
||||||
### Screenshots
|
### Screenshots
|
||||||
|
|
||||||
|
|||||||
+7
-3
@@ -105,7 +105,7 @@ real-time-коллаборации Docmost, поэтому запись нико
|
|||||||
- ✅ **Шаблоны страниц** — пометить страницу шаблоном и вставлять её содержимое живой ссылкой в другие страницы; правки шаблона распространяются на все места вставки (whole-page-транслюзия поверх существующих synced-блоков).
|
- ✅ **Шаблоны страниц** — пометить страницу шаблоном и вставлять её содержимое живой ссылкой в другие страницы; правки шаблона распространяются на все места вставки (whole-page-транслюзия поверх существующих synced-блоков).
|
||||||
- ✅ **AI-ассистент на публичных шарах** — анонимный зритель расшаренной страницы может спросить AI-агента, который ищет строго по дереву этой шары (read-only, share-scoped поиск), за тумблером воркспейса.
|
- ✅ **AI-ассистент на публичных шарах** — анонимный зритель расшаренной страницы может спросить AI-агента, который ищет строго по дереву этой шары (read-only, share-scoped поиск), за тумблером воркспейса.
|
||||||
- ✅ **Сноски** — сноски академического вида: нумерованная ссылка-надстрочник прямо в тексте (читается на месте во всплывающем окне по наведению), а текст сноски живёт реальным редактируемым блоком внизу страницы; авто-нумерация, безопасна для совместного редактирования, переживает экспорт/импорт Markdown и доступна AI-агенту / MCP.
|
- ✅ **Сноски** — сноски академического вида: нумерованная ссылка-надстрочник прямо в тексте (читается на месте во всплывающем окне по наведению), а текст сноски живёт реальным редактируемым блоком внизу страницы; авто-нумерация, безопасна для совместного редактирования, переживает экспорт/импорт Markdown и доступна AI-агенту / MCP.
|
||||||
- ✅ **Временные заметки** — пометьте заметку временной, и она автоматически уедет в корзину по истечении настраиваемого срока жизни воркспейса (по умолчанию 24 ч), если её предварительно не сделать постоянной; создать такую можно в один клик с домашнего экрана, с обзора любого пространства или из сайдбара пространства, а на открытой заметке есть баннер «Сделать постоянной».
|
- ✅ **Временные заметки** — создайте временную заметку, и она автоматически уедет в корзину по истечении настраиваемого срока жизни (по умолчанию 24 ч); создать такую можно в один клик с домашнего экрана, с обзора любого пространства или из сайдбара пространства.
|
||||||
|
|
||||||
### В процессе
|
### В процессе
|
||||||
|
|
||||||
@@ -174,14 +174,18 @@ dump/restore, существующий каталог данных переис
|
|||||||
- Пространства (Spaces)
|
- Пространства (Spaces)
|
||||||
- Управление правами доступа
|
- Управление правами доступа
|
||||||
- Группы
|
- Группы
|
||||||
- Комментарии (с резолвом / переоткрытием)
|
- Комментарии (с резолвом / переоткрытием и всплывающими подсказками с текстом комментария при наведении)
|
||||||
- История страниц
|
- История страниц
|
||||||
- Поиск
|
- Поиск
|
||||||
- Вложения файлов
|
- Вложения файлов
|
||||||
- Встраивания (Airtable, Loom, Miro и другие)
|
- Встраивания (Airtable, Loom, Miro и другие)
|
||||||
- Переводы (10+ языков)
|
- Переводы (10+ языков)
|
||||||
- Встроенный MCP-сервер (`/mcp`)
|
- Встроенный MCP-сервер (`/mcp`)
|
||||||
- Чат с AI-агентом по вики (чтение + запись, RAG-поиск, внешние MCP / доступ в интернет)
|
- Чат с AI-агентом по вики (чтение + запись, RAG-поиск, внешние MCP / доступ в интернет); окно чата закрепляется в боковом меню, а агент узнаёт о ваших правках страницы между ходами
|
||||||
|
- Кнопки код-блока оверлеем, селектор языка появляется при наведении
|
||||||
|
- Кнопка «Ударение» (U+0301) в bubble-меню
|
||||||
|
- Позиция чтения (прокрутка) восстанавливается после перезагрузки
|
||||||
|
- Slash-меню терпимо к неправильной раскладке (ЙЦУКЕН↔QWERTY)
|
||||||
|
|
||||||
### Скриншоты
|
### Скриншоты
|
||||||
|
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import { acceptInvitation } from "@/features/workspace/services/workspace-servic
|
|||||||
import APP_ROUTE, { getPostLoginRedirect } from "@/lib/app-route.ts";
|
import APP_ROUTE, { getPostLoginRedirect } from "@/lib/app-route.ts";
|
||||||
import { RESET } from "jotai/utils";
|
import { RESET } from "jotai/utils";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
|
import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom";
|
||||||
|
|
||||||
export default function useAuth() {
|
export default function useAuth() {
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
@@ -122,6 +123,11 @@ export default function useAuth() {
|
|||||||
|
|
||||||
const handleLogout = async () => {
|
const handleLogout = async () => {
|
||||||
setCurrentUser(RESET);
|
setCurrentUser(RESET);
|
||||||
|
// Purge the persisted sidebar tree caches (they contain page titles) so the
|
||||||
|
// cached page titles aren't left readable in localStorage on a shared
|
||||||
|
// machine. (Only the tree caches are swept; other localStorage entries
|
||||||
|
// remain.)
|
||||||
|
clearPersistedTreeCaches();
|
||||||
await logout();
|
await logout();
|
||||||
window.location.replace(`${APP_ROUTE.AUTH.LOGIN}?logout=1`);
|
window.location.replace(`${APP_ROUTE.AUTH.LOGIN}?logout=1`);
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -0,0 +1,231 @@
|
|||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { Editor } from "@tiptap/core";
|
||||||
|
import { Document } from "@tiptap/extension-document";
|
||||||
|
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||||
|
import { Text } from "@tiptap/extension-text";
|
||||||
|
import { ySyncPluginKey } from "@tiptap/y-tiptap";
|
||||||
|
import {
|
||||||
|
CustomTypography,
|
||||||
|
undoGuardKey,
|
||||||
|
findChangedRange,
|
||||||
|
mapRangeThroughChange,
|
||||||
|
} from "./custom-typography";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* PR #296 — the collab-safe typography undo-guard is exercised through the REAL
|
||||||
|
* editor path: a fresh Editor with the CustomTypography extension, transactions
|
||||||
|
* tagged exactly the way prosemirror-history / y-tiptap tag undo & remote
|
||||||
|
* changes (`setMeta("history$", …)` and `setMeta(ySyncPluginKey, …)`), plus
|
||||||
|
* direct unit tests of the two pure diff helpers. No hand-poke of plugin state.
|
||||||
|
*
|
||||||
|
* ARMING MECHANISM (verified against custom-typography.ts source):
|
||||||
|
* - A transaction arms the guard only when it is BOTH history/remote
|
||||||
|
* (`getMeta("history$")` truthy, or `isChangeOrigin` via the ySync meta)
|
||||||
|
* AND an undo/redo (`getMeta("history$")` truthy, or ySync
|
||||||
|
* `isUndoRedoOperation`), AND its whole-doc diff is a REPLACE
|
||||||
|
* (change.oldTo > change.from && change.newTo > change.from).
|
||||||
|
* - `history$` is the stringified PluginKey of the single prosemirror-history
|
||||||
|
* plugin; ProseMirror stores meta under `key.key`, so setMeta("history$")
|
||||||
|
* in a test is read identically by the extension's getMeta("history$").
|
||||||
|
*/
|
||||||
|
|
||||||
|
const singlePara = (text: string) => ({
|
||||||
|
type: "doc",
|
||||||
|
content: [{ type: "paragraph", content: [{ type: "text", text }] }],
|
||||||
|
});
|
||||||
|
|
||||||
|
const makeEditor = (text: string) =>
|
||||||
|
new Editor({
|
||||||
|
extensions: [Document, Paragraph, Text, CustomTypography],
|
||||||
|
content: singlePara(text),
|
||||||
|
});
|
||||||
|
|
||||||
|
// Build a before/after EditorState pair by applying one plain transaction.
|
||||||
|
const mutate = (text: string, apply: (tr: any, schema: any) => void) => {
|
||||||
|
const editor = new Editor({
|
||||||
|
extensions: [Document, Paragraph, Text],
|
||||||
|
content: singlePara(text),
|
||||||
|
});
|
||||||
|
const before = editor.state;
|
||||||
|
const tr = before.tr;
|
||||||
|
apply(tr, before.schema);
|
||||||
|
editor.view.dispatch(tr);
|
||||||
|
const after = editor.state;
|
||||||
|
return { before, after, editor };
|
||||||
|
};
|
||||||
|
|
||||||
|
describe("findChangedRange", () => {
|
||||||
|
it("returns null for identical docs", () => {
|
||||||
|
const editor = new Editor({
|
||||||
|
extensions: [Document, Paragraph, Text],
|
||||||
|
content: singlePara("hello"),
|
||||||
|
});
|
||||||
|
expect(findChangedRange(editor.state, editor.state)).toBeNull();
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns the minimal range for a normal middle insertion", () => {
|
||||||
|
// "hello world" (text at 1..12); insert "there " at pos 6.
|
||||||
|
const { before, after, editor } = mutate("hello world", (tr) =>
|
||||||
|
tr.insertText("there ", 6),
|
||||||
|
);
|
||||||
|
expect(findChangedRange(before, after)).toEqual({
|
||||||
|
from: 6,
|
||||||
|
oldTo: 6,
|
||||||
|
newTo: 12,
|
||||||
|
});
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes the INSERTION overlapping-bounds branch (repeated content)", () => {
|
||||||
|
// Insert one more 'a' into "aaaaa" at pos 3. findDiffStart lands at the end
|
||||||
|
// (6) while findDiffEnd reports an end BEFORE it ({a:1,b:2}); both ends must
|
||||||
|
// be pushed forward by the same delta -> a non-degenerate range.
|
||||||
|
const { before, after, editor } = mutate("aaaaa", (tr) =>
|
||||||
|
tr.insertText("a", 3),
|
||||||
|
);
|
||||||
|
const change = findChangedRange(before, after)!;
|
||||||
|
expect(change).toEqual({ from: 6, oldTo: 6, newTo: 7 });
|
||||||
|
// Invariant the guard logic relies on: never degenerate.
|
||||||
|
expect(change.from).toBeLessThanOrEqual(change.oldTo);
|
||||||
|
expect(change.from).toBeLessThanOrEqual(change.newTo);
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes the DELETION overlapping-bounds branch (F2 fix)", () => {
|
||||||
|
// Delete one repeated 'a' from the middle of "aaaaa" ([3,4)). Here
|
||||||
|
// findDiffEnd reports newTo < start, the symmetric case the old one-sided
|
||||||
|
// normalization missed -> it used to yield a degenerate range (newTo < from).
|
||||||
|
const { before, after, editor } = mutate("aaaaa", (tr) => tr.delete(3, 4));
|
||||||
|
const change = findChangedRange(before, after)!;
|
||||||
|
expect(change).toEqual({ from: 5, oldTo: 6, newTo: 5 });
|
||||||
|
// The whole point of F2: from <= newTo (and from <= oldTo) still holds.
|
||||||
|
expect(change.from).toBeLessThanOrEqual(change.newTo);
|
||||||
|
expect(change.from).toBeLessThanOrEqual(change.oldTo);
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalizes a multi-char repeated deletion (F2 fix)", () => {
|
||||||
|
const { before, after, editor } = mutate("aaaaa", (tr) => tr.delete(2, 4));
|
||||||
|
const change = findChangedRange(before, after)!;
|
||||||
|
expect(change).toEqual({ from: 4, oldTo: 6, newTo: 4 });
|
||||||
|
expect(change.from).toBeLessThanOrEqual(change.newTo);
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("mapRangeThroughChange", () => {
|
||||||
|
const range = { from: 5, to: 10 };
|
||||||
|
|
||||||
|
it("RELEASES on a strict intersection (edit inside the guarded range)", () => {
|
||||||
|
// change straddles the interior of the guard.
|
||||||
|
expect(
|
||||||
|
mapRangeThroughChange(range, { from: 6, oldTo: 8, newTo: 7 }),
|
||||||
|
).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT release on a boundary touch at the guard END", () => {
|
||||||
|
// Edit begins exactly at range.to (10): from < to is false -> no intersect.
|
||||||
|
expect(
|
||||||
|
mapRangeThroughChange(range, { from: 10, oldTo: 10, newTo: 12 }),
|
||||||
|
).toEqual(range);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT release on a boundary touch at the guard START", () => {
|
||||||
|
// Edit ends exactly at range.from (5): oldTo > from is false -> no intersect;
|
||||||
|
// it is treated as a change fully before, shifting the guard.
|
||||||
|
expect(
|
||||||
|
mapRangeThroughChange(range, { from: 3, oldTo: 5, newTo: 8 }),
|
||||||
|
).toEqual({ from: 8, to: 13 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("SHIFTS the guard for a change fully before it", () => {
|
||||||
|
// Insert 2 chars entirely before the range (oldTo 3 <= from 5): +2 delta.
|
||||||
|
expect(
|
||||||
|
mapRangeThroughChange(range, { from: 2, oldTo: 3, newTo: 5 }),
|
||||||
|
).toEqual({ from: 7, to: 12 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the guard untouched for a change fully after it", () => {
|
||||||
|
expect(
|
||||||
|
mapRangeThroughChange(range, { from: 12, oldTo: 14, newTo: 16 }),
|
||||||
|
).toBe(range);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("undo-guard arming (integration)", () => {
|
||||||
|
it("arms {from, to:newTo} on a LOCAL undo-replace (history meta)", () => {
|
||||||
|
// Undo of an em-dash substitution: "a—b" restored to "a--b" — the em-dash
|
||||||
|
// (pos 2..3) is REPLACED by "--", tagged with the history plugin's meta.
|
||||||
|
const editor = makeEditor("a—b");
|
||||||
|
const { state } = editor;
|
||||||
|
const tr = state.tr
|
||||||
|
.replaceWith(2, 3, state.schema.text("--"))
|
||||||
|
.setMeta("history$", { redo: false });
|
||||||
|
editor.view.dispatch(tr);
|
||||||
|
|
||||||
|
expect(editor.state.doc.textContent).toBe("a--b");
|
||||||
|
// from = diff start (2), to = newTo = end of the inserted "--" (4).
|
||||||
|
expect(undoGuardKey.getState(editor.state)).toEqual({ from: 2, to: 4 });
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT arm on a REMOTE change-origin replace (no undo meta)", () => {
|
||||||
|
// Same replace, but tagged only as a y-sync remote change: history/remote
|
||||||
|
// yes, undo/redo NO -> must not arm.
|
||||||
|
const editor = makeEditor("a—b");
|
||||||
|
const { state } = editor;
|
||||||
|
const tr = state.tr
|
||||||
|
.replaceWith(2, 3, state.schema.text("--"))
|
||||||
|
.setMeta(ySyncPluginKey, { isChangeOrigin: true });
|
||||||
|
editor.view.dispatch(tr);
|
||||||
|
|
||||||
|
expect(editor.state.doc.textContent).toBe("a--b");
|
||||||
|
expect(undoGuardKey.getState(editor.state)).toBeNull();
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT arm on an ordinary local edit", () => {
|
||||||
|
const editor = makeEditor("a—b");
|
||||||
|
editor.view.dispatch(
|
||||||
|
editor.state.tr.replaceWith(2, 3, editor.state.schema.text("--")),
|
||||||
|
);
|
||||||
|
expect(undoGuardKey.getState(editor.state)).toBeNull();
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("undo-guard release / shift (integration)", () => {
|
||||||
|
it("RELEASES when a later edit lands inside the guarded region", () => {
|
||||||
|
const editor = makeEditor("a—b");
|
||||||
|
editor.view.dispatch(
|
||||||
|
editor.state.tr
|
||||||
|
.replaceWith(2, 3, editor.state.schema.text("--"))
|
||||||
|
.setMeta("history$", { redo: false }),
|
||||||
|
);
|
||||||
|
const guard = undoGuardKey.getState(editor.state)!;
|
||||||
|
expect(guard).toEqual({ from: 2, to: 4 });
|
||||||
|
|
||||||
|
// Type a character inside the restored region -> guard is dropped.
|
||||||
|
editor.view.dispatch(editor.state.tr.insertText("x", guard.from + 1));
|
||||||
|
expect(undoGuardKey.getState(editor.state)).toBeNull();
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps and SHIFTS the guard when a later edit lands before it", () => {
|
||||||
|
const editor = makeEditor("zz a—b");
|
||||||
|
// "zz a—b": em-dash at pos 5; replace the 'a' at 4..5 with "--" to arm.
|
||||||
|
editor.view.dispatch(
|
||||||
|
editor.state.tr
|
||||||
|
.replaceWith(4, 5, editor.state.schema.text("--"))
|
||||||
|
.setMeta("history$", { redo: false }),
|
||||||
|
);
|
||||||
|
const guard = undoGuardKey.getState(editor.state)!;
|
||||||
|
expect(guard).toEqual({ from: 4, to: 6 });
|
||||||
|
|
||||||
|
// Insert one char at the very start (before the guard) -> guard shifts +1.
|
||||||
|
editor.view.dispatch(editor.state.tr.insertText("Q", 1));
|
||||||
|
expect(undoGuardKey.getState(editor.state)).toEqual({ from: 5, to: 7 });
|
||||||
|
editor.destroy();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,193 @@
|
|||||||
|
import { InputRule } from "@tiptap/core";
|
||||||
|
import {
|
||||||
|
Plugin,
|
||||||
|
PluginKey,
|
||||||
|
type EditorState,
|
||||||
|
type Transaction,
|
||||||
|
} from "@tiptap/pm/state";
|
||||||
|
import { Typography } from "@tiptap/extension-typography";
|
||||||
|
import { isChangeOrigin } from "@tiptap/extension-collaboration";
|
||||||
|
import { ySyncPluginKey } from "@tiptap/y-tiptap";
|
||||||
|
|
||||||
|
// Region restored by the latest undo — while it is intact, typography
|
||||||
|
// input rules overlapping it must not fire again.
|
||||||
|
interface UndoGuardRange {
|
||||||
|
from: number;
|
||||||
|
to: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Exported for tests: the plugin key lets a test read the armed guard state,
|
||||||
|
// and the two pure helpers below are unit-tested directly.
|
||||||
|
export const undoGuardKey = new PluginKey<UndoGuardRange | null>(
|
||||||
|
"typographyUndoGuard",
|
||||||
|
);
|
||||||
|
|
||||||
|
// prosemirror-history does not export its plugin key, so template-editor
|
||||||
|
// undo/redo is detected via the stable stringified key. Only one
|
||||||
|
// PluginKey("history") exists in the dependency tree, so "history$" is stable.
|
||||||
|
const HISTORY_META = "history$";
|
||||||
|
|
||||||
|
const isUndoRedoTransaction = (tr: Transaction): boolean => {
|
||||||
|
if (tr.getMeta(HISTORY_META)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
// Read yjs undo/redo meta via the real ySyncPluginKey object (imported, not
|
||||||
|
// a fragile stringified key), which y-tiptap sets on Y.UndoManager changes.
|
||||||
|
const ySyncMeta = tr.getMeta(ySyncPluginKey) as
|
||||||
|
| { isUndoRedoOperation?: boolean }
|
||||||
|
| undefined;
|
||||||
|
return !!ySyncMeta?.isUndoRedoOperation;
|
||||||
|
};
|
||||||
|
|
||||||
|
interface DocChange {
|
||||||
|
from: number;
|
||||||
|
oldTo: number;
|
||||||
|
newTo: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Compute the minimal changed region between two docs. yjs undo/redo (and any
|
||||||
|
// remote change) arrives as a whole-document replace step, so the transaction
|
||||||
|
// step maps are useless — diff the docs to recover the real minimal change.
|
||||||
|
// Returns null when the docs are identical.
|
||||||
|
export const findChangedRange = (
|
||||||
|
oldState: EditorState,
|
||||||
|
newState: EditorState,
|
||||||
|
): DocChange | null => {
|
||||||
|
const start = oldState.doc.content.findDiffStart(newState.doc.content);
|
||||||
|
const end = oldState.doc.content.findDiffEnd(newState.doc.content);
|
||||||
|
if (start == null || end == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
let { a: oldTo, b: newTo } = end;
|
||||||
|
// findDiffEnd can report an end BEFORE the diff start when the changed text
|
||||||
|
// abuts repeated content (insertion -> oldTo<start, deletion -> newTo<start).
|
||||||
|
// Push both ends forward by the same delta so the range stays non-degenerate
|
||||||
|
// (from <= oldTo and from <= newTo), matching ProseMirror's own diff bounds.
|
||||||
|
const minTo = Math.min(oldTo, newTo);
|
||||||
|
if (minTo < start) {
|
||||||
|
const delta = start - minTo;
|
||||||
|
oldTo += delta;
|
||||||
|
newTo += delta;
|
||||||
|
}
|
||||||
|
return { from: start, oldTo, newTo };
|
||||||
|
};
|
||||||
|
|
||||||
|
// Map an armed guard range across a single document change described by a diff.
|
||||||
|
// Returns null when the change touches the guarded text itself (the restored
|
||||||
|
// substitution was edited, so the guard must be released).
|
||||||
|
export const mapRangeThroughChange = (
|
||||||
|
range: UndoGuardRange,
|
||||||
|
change: DocChange,
|
||||||
|
): UndoGuardRange | null => {
|
||||||
|
// Strict intersection: an edit exactly at a guard boundary (e.g. the user
|
||||||
|
// typing the suppressed space right after the restored text, or deleting it)
|
||||||
|
// must NOT drop the guard.
|
||||||
|
if (change.from < range.to && change.oldTo > range.from) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
// Change fully before the guard: shift the guard by the length delta.
|
||||||
|
if (change.oldTo <= range.from) {
|
||||||
|
const delta = change.newTo - change.oldTo;
|
||||||
|
return { from: range.from + delta, to: range.to + delta };
|
||||||
|
}
|
||||||
|
// Change fully after the guard: positions are unaffected.
|
||||||
|
return range;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Detect history/remote transactions that may arrive as a whole-document
|
||||||
|
// replace step: prosemirror-history undo/redo, or any yjs remote-origin change
|
||||||
|
// (isChangeOrigin is the canonical predicate already used across the app).
|
||||||
|
const isHistoryOrRemoteTransaction = (tr: Transaction): boolean =>
|
||||||
|
!!tr.getMeta(HISTORY_META) || isChangeOrigin(tr);
|
||||||
|
|
||||||
|
export const CustomTypography = Typography.extend({
|
||||||
|
addProseMirrorPlugins() {
|
||||||
|
return [
|
||||||
|
...(this.parent?.() ?? []),
|
||||||
|
new Plugin({
|
||||||
|
key: undoGuardKey,
|
||||||
|
state: {
|
||||||
|
init: () => null,
|
||||||
|
apply(tr, prev, oldState, newState): UndoGuardRange | null {
|
||||||
|
if (tr.docChanged && isHistoryOrRemoteTransaction(tr)) {
|
||||||
|
const change = findChangedRange(oldState, newState);
|
||||||
|
if (change == null) {
|
||||||
|
// Attribute-only or otherwise content-neutral change: keep the
|
||||||
|
// guard.
|
||||||
|
return prev;
|
||||||
|
}
|
||||||
|
// Arm the guard only when the LOCAL user's undo/redo REPLACED text
|
||||||
|
// (deleted + inserted) — the signature of reverting an input-rule
|
||||||
|
// substitution. Pure insertions/deletions and remote peer edits
|
||||||
|
// must not arm it.
|
||||||
|
if (
|
||||||
|
isUndoRedoTransaction(tr) &&
|
||||||
|
change.oldTo > change.from &&
|
||||||
|
change.newTo > change.from
|
||||||
|
) {
|
||||||
|
return { from: change.from, to: change.newTo };
|
||||||
|
}
|
||||||
|
// Non-arming history/remote change: map the existing guard through
|
||||||
|
// the real diff instead of the (whole-document) step map.
|
||||||
|
if (!prev) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return mapRangeThroughChange(prev, change);
|
||||||
|
}
|
||||||
|
if (!prev) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
if (!tr.docChanged) {
|
||||||
|
return prev;
|
||||||
|
}
|
||||||
|
// Ordinary local edit: minimal step maps are accurate and cheap.
|
||||||
|
let range: UndoGuardRange | null = prev;
|
||||||
|
for (const stepMap of tr.mapping.maps) {
|
||||||
|
const { from: rangeFrom, to: rangeTo } = range;
|
||||||
|
let touched = false;
|
||||||
|
stepMap.forEach((fromA, toA) => {
|
||||||
|
if (fromA < rangeTo && toA > rangeFrom) {
|
||||||
|
touched = true;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
if (touched) {
|
||||||
|
range = null;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
range = {
|
||||||
|
from: stepMap.map(rangeFrom, 1),
|
||||||
|
to: stepMap.map(rangeTo, -1),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return range && range.to > range.from ? range : null;
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
},
|
||||||
|
|
||||||
|
addInputRules() {
|
||||||
|
// Wrap every typography rule: skip it when its match overlaps the text
|
||||||
|
// just restored by undo, so an undone substitution is not re-applied.
|
||||||
|
return (this.parent?.() ?? []).map(
|
||||||
|
(rule) =>
|
||||||
|
new InputRule({
|
||||||
|
find: rule.find,
|
||||||
|
undoable: rule.undoable,
|
||||||
|
handler: (props) => {
|
||||||
|
const guard = undoGuardKey.getState(props.state);
|
||||||
|
if (
|
||||||
|
guard &&
|
||||||
|
props.range.from < guard.to &&
|
||||||
|
props.range.to > guard.from
|
||||||
|
) {
|
||||||
|
// Returning null skips this rule and lets the typed character
|
||||||
|
// be inserted as plain text.
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return rule.handler(props);
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
});
|
||||||
@@ -6,7 +6,7 @@ import { TaskList, TaskItem } from "@tiptap/extension-list";
|
|||||||
import { Placeholder, CharacterCount, UndoRedo } from "@tiptap/extensions";
|
import { Placeholder, CharacterCount, UndoRedo } from "@tiptap/extensions";
|
||||||
import { Superscript } from "@tiptap/extension-superscript";
|
import { Superscript } from "@tiptap/extension-superscript";
|
||||||
import SubScript from "@tiptap/extension-subscript";
|
import SubScript from "@tiptap/extension-subscript";
|
||||||
import { Typography } from "@tiptap/extension-typography";
|
import { CustomTypography } from "./custom-typography";
|
||||||
import { TextStyle } from "@tiptap/extension-text-style";
|
import { TextStyle } from "@tiptap/extension-text-style";
|
||||||
import { Color } from "@tiptap/extension-color";
|
import { Color } from "@tiptap/extension-color";
|
||||||
import { Youtube } from "@tiptap/extension-youtube";
|
import { Youtube } from "@tiptap/extension-youtube";
|
||||||
@@ -245,7 +245,9 @@ export const mainExtensions = [
|
|||||||
return ReactMarkViewRenderer(SpoilerView);
|
return ReactMarkViewRenderer(SpoilerView);
|
||||||
},
|
},
|
||||||
}),
|
}),
|
||||||
Typography,
|
// Typography with an undo guard: does not re-apply a substitution the user
|
||||||
|
// just undid (e.g. Ctrl+Z on "1/2" -> "½" followed by another space).
|
||||||
|
CustomTypography,
|
||||||
TrailingNode,
|
TrailingNode,
|
||||||
GlobalDragHandle.configure({
|
GlobalDragHandle.configure({
|
||||||
customNodes: ["transclusionSource", "transclusionReference", "pageEmbed"],
|
customNodes: ["transclusionSource", "transclusionReference", "pageEmbed"],
|
||||||
|
|||||||
@@ -1,5 +1,9 @@
|
|||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect } from "vitest";
|
||||||
import { normalizeTableColumnWidths } from "./markdown-clipboard";
|
import { htmlToMarkdown } from "@docmost/editor-ext";
|
||||||
|
import {
|
||||||
|
normalizeTableColumnWidths,
|
||||||
|
classifyClipboardSelection,
|
||||||
|
} from "./markdown-clipboard";
|
||||||
|
|
||||||
// normalizeTableColumnWidths mutates a DOM subtree (jsdom provides document).
|
// normalizeTableColumnWidths mutates a DOM subtree (jsdom provides document).
|
||||||
function root(html: string): HTMLElement {
|
function root(html: string): HTMLElement {
|
||||||
@@ -124,3 +128,171 @@ describe("normalizeTableColumnWidths", () => {
|
|||||||
).toEqual([null, null]);
|
).toEqual([null, null]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("classifyClipboardSelection", () => {
|
||||||
|
it("serializes a list of 2+ items as markdown", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([{ name: "bulletList", childCount: 2 }]),
|
||||||
|
).toEqual({ asMarkdown: true, wrapBareRows: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves a single-item list as plain text", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([{ name: "bulletList", childCount: 1 }]),
|
||||||
|
).toEqual({ asMarkdown: false, wrapBareRows: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("serializes a whole table without wrapping bare rows", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([{ name: "table", childCount: 3 }]),
|
||||||
|
).toEqual({ asMarkdown: true, wrapBareRows: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("serializes a partial cell selection (bare rows) and flags wrapping", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([
|
||||||
|
{ name: "tableRow", childCount: 2 },
|
||||||
|
{ name: "tableRow", childCount: 2 },
|
||||||
|
]),
|
||||||
|
).toEqual({ asMarkdown: true, wrapBareRows: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves plain paragraphs as plain text", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([{ name: "paragraph", childCount: 1 }]),
|
||||||
|
).toEqual({ asMarkdown: false, wrapBareRows: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not wrap when rows are mixed with other block types", () => {
|
||||||
|
expect(
|
||||||
|
classifyClipboardSelection([
|
||||||
|
{ name: "tableRow", childCount: 2 },
|
||||||
|
{ name: "paragraph", childCount: 1 },
|
||||||
|
]),
|
||||||
|
).toEqual({ asMarkdown: false, wrapBareRows: false });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// Output-level tests for the table clipboard regression: copying a table must
|
||||||
|
// yield a real GFM pipe table, NOT one-value-per-line concatenated cells.
|
||||||
|
// These exercise the actual markdown produced by htmlToMarkdown (the same
|
||||||
|
// serializer step the clipboardTextSerializer runs), so they pin the OUTPUT
|
||||||
|
// shape that the classifier-flag tests above do not cover.
|
||||||
|
describe("table clipboard markdown output (htmlToMarkdown)", () => {
|
||||||
|
// Trim each line and drop blanks so structural assertions are whitespace-robust.
|
||||||
|
function lines(md: string): string[] {
|
||||||
|
return md
|
||||||
|
.split("\n")
|
||||||
|
.map((l) => l.trim())
|
||||||
|
.filter((l) => l.length > 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
// A GFM separator row like "| --- | --- |" (any number of columns), tolerant
|
||||||
|
// of the padding turndown emits.
|
||||||
|
function isSeparatorRow(line: string): boolean {
|
||||||
|
const compact = line.replace(/\s+/g, "");
|
||||||
|
return /^\|(?:-{3,}\|)+$/.test(compact);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Split a pipe-delimited row into trimmed cell values.
|
||||||
|
function cells(line: string): string[] {
|
||||||
|
return line
|
||||||
|
.replace(/^\|/, "")
|
||||||
|
.replace(/\|$/, "")
|
||||||
|
.split("|")
|
||||||
|
.map((c) => c.trim());
|
||||||
|
}
|
||||||
|
|
||||||
|
it("serializes a header-less partial cell selection (bare rows) as a valid GFM pipe table", () => {
|
||||||
|
// Mirror the serializer's `wrapBareRows` branch exactly: bare <tr> nodes are
|
||||||
|
// wrapped in <table><tbody> and htmlToMarkdown(div.innerHTML) is called.
|
||||||
|
// See markdown-clipboard.ts clipboardTextSerializer:
|
||||||
|
// const table = document.createElement("table");
|
||||||
|
// const tbody = document.createElement("tbody");
|
||||||
|
// tbody.appendChild(fragment); table.appendChild(tbody);
|
||||||
|
// div.appendChild(table);
|
||||||
|
// return htmlToMarkdown(div.innerHTML);
|
||||||
|
const div = document.createElement("div");
|
||||||
|
const table = document.createElement("table");
|
||||||
|
const tbody = document.createElement("tbody");
|
||||||
|
for (const [c1, c2] of [
|
||||||
|
["a", "b"],
|
||||||
|
["c", "d"],
|
||||||
|
]) {
|
||||||
|
const tr = document.createElement("tr");
|
||||||
|
const td1 = document.createElement("td");
|
||||||
|
td1.textContent = c1;
|
||||||
|
const td2 = document.createElement("td");
|
||||||
|
td2.textContent = c2;
|
||||||
|
tr.appendChild(td1);
|
||||||
|
tr.appendChild(td2);
|
||||||
|
tbody.appendChild(tr);
|
||||||
|
}
|
||||||
|
table.appendChild(tbody);
|
||||||
|
div.appendChild(table);
|
||||||
|
|
||||||
|
const md = htmlToMarkdown(div.innerHTML);
|
||||||
|
const ls = lines(md);
|
||||||
|
|
||||||
|
// Valid GFM: a header/data separator row is present (an empty header is
|
||||||
|
// synthesized by the GFM turndown plugin for a header-less table — fine).
|
||||||
|
expect(ls.some(isSeparatorRow)).toBe(true);
|
||||||
|
// NOT the old broken "one value per line" shape: every line is pipe-delimited
|
||||||
|
// and no line is a bare cell value on its own.
|
||||||
|
expect(ls.every((l) => l.includes("|"))).toBe(true);
|
||||||
|
expect(md).not.toMatch(/^\s*(a|b|c|d)\s*$/m);
|
||||||
|
// The cell values land in real pipe-delimited data rows.
|
||||||
|
const dataRows = ls.filter((l) => !isSeparatorRow(l)).map(cells);
|
||||||
|
expect(dataRows).toContainEqual(["a", "b"]);
|
||||||
|
expect(dataRows).toContainEqual(["c", "d"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("serializes a whole table with a header row as a proper GFM table (headline regression)", () => {
|
||||||
|
// Mirror the serializer's non-wrap branch: the full <table> node is appended
|
||||||
|
// directly (div.appendChild(fragment)) and htmlToMarkdown(div.innerHTML) runs.
|
||||||
|
const div = document.createElement("div");
|
||||||
|
const table = document.createElement("table");
|
||||||
|
|
||||||
|
const thead = document.createElement("thead");
|
||||||
|
const headerRow = document.createElement("tr");
|
||||||
|
for (const h of ["Name", "Age"]) {
|
||||||
|
const th = document.createElement("th");
|
||||||
|
th.textContent = h;
|
||||||
|
headerRow.appendChild(th);
|
||||||
|
}
|
||||||
|
thead.appendChild(headerRow);
|
||||||
|
table.appendChild(thead);
|
||||||
|
|
||||||
|
const tbody = document.createElement("tbody");
|
||||||
|
for (const [name, age] of [
|
||||||
|
["Alice", "30"],
|
||||||
|
["Bob", "25"],
|
||||||
|
]) {
|
||||||
|
const tr = document.createElement("tr");
|
||||||
|
const td1 = document.createElement("td");
|
||||||
|
td1.textContent = name;
|
||||||
|
const td2 = document.createElement("td");
|
||||||
|
td2.textContent = age;
|
||||||
|
tr.appendChild(td1);
|
||||||
|
tr.appendChild(td2);
|
||||||
|
tbody.appendChild(tr);
|
||||||
|
}
|
||||||
|
table.appendChild(tbody);
|
||||||
|
div.appendChild(table);
|
||||||
|
|
||||||
|
const md = htmlToMarkdown(div.innerHTML);
|
||||||
|
const ls = lines(md);
|
||||||
|
|
||||||
|
// Proper GFM structure: separator row + all rows pipe-delimited.
|
||||||
|
expect(ls.some(isSeparatorRow)).toBe(true);
|
||||||
|
expect(ls.every((l) => l.includes("|"))).toBe(true);
|
||||||
|
|
||||||
|
const rows = ls.filter((l) => !isSeparatorRow(l)).map(cells);
|
||||||
|
// Header row comes first, followed by both data rows.
|
||||||
|
expect(rows[0]).toEqual(["Name", "Age"]);
|
||||||
|
expect(rows).toContainEqual(["Alice", "30"]);
|
||||||
|
expect(rows).toContainEqual(["Bob", "25"]);
|
||||||
|
// Headline regression: the table is NOT concatenated one-value-per-line.
|
||||||
|
expect(md).not.toMatch(/^\s*(Name|Age|Alice|Bob|30|25)\s*$/m);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -27,24 +27,36 @@ export const MarkdownClipboard = Extension.create({
|
|||||||
key: new PluginKey("markdownClipboard"),
|
key: new PluginKey("markdownClipboard"),
|
||||||
props: {
|
props: {
|
||||||
clipboardTextSerializer: (slice) => {
|
clipboardTextSerializer: (slice) => {
|
||||||
const listTypes = ["bulletList", "orderedList", "taskList"];
|
const topLevelNodes: { name: string; childCount: number }[] = [];
|
||||||
let topLevelCount = 0;
|
|
||||||
let hasList = false;
|
|
||||||
slice.content.forEach((node) => {
|
slice.content.forEach((node) => {
|
||||||
if (listTypes.includes(node.type.name)) {
|
topLevelNodes.push({
|
||||||
hasList = true;
|
name: node.type.name,
|
||||||
topLevelCount += node.childCount;
|
childCount: node.childCount,
|
||||||
} else {
|
});
|
||||||
topLevelCount++;
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!hasList || topLevelCount < 2) return null;
|
const { asMarkdown, wrapBareRows } =
|
||||||
|
classifyClipboardSelection(topLevelNodes);
|
||||||
|
if (!asMarkdown) return null;
|
||||||
|
|
||||||
const div = document.createElement("div");
|
const div = document.createElement("div");
|
||||||
const serializer = DOMSerializer.fromSchema(this.editor.schema);
|
const serializer = DOMSerializer.fromSchema(this.editor.schema);
|
||||||
const fragment = serializer.serializeFragment(slice.content);
|
const fragment = serializer.serializeFragment(slice.content);
|
||||||
div.appendChild(fragment);
|
|
||||||
|
if (wrapBareRows) {
|
||||||
|
// A partial table cell-selection serializes to bare <tr> nodes
|
||||||
|
// (prosemirror-tables returns the whole `table` node only when the
|
||||||
|
// entire table is selected). Bare <tr> would be foster-parented
|
||||||
|
// away by the HTML parser inside htmlToMarkdown, so wrap them in
|
||||||
|
// <table><tbody> first for the GFM turndown rule to detect them.
|
||||||
|
const table = document.createElement("table");
|
||||||
|
const tbody = document.createElement("tbody");
|
||||||
|
tbody.appendChild(fragment);
|
||||||
|
table.appendChild(tbody);
|
||||||
|
div.appendChild(table);
|
||||||
|
} else {
|
||||||
|
div.appendChild(fragment);
|
||||||
|
}
|
||||||
return htmlToMarkdown(div.innerHTML);
|
return htmlToMarkdown(div.innerHTML);
|
||||||
},
|
},
|
||||||
handlePaste: (view, event, slice) => {
|
handlePaste: (view, event, slice) => {
|
||||||
@@ -153,6 +165,55 @@ export const MarkdownClipboard = Extension.create({
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide whether a copied slice's plain-text clipboard payload should be
|
||||||
|
* serialized as Markdown (instead of ProseMirror's default text serializer,
|
||||||
|
* which joins block leaves with newlines — the "one value per line" bug for
|
||||||
|
* tables).
|
||||||
|
*
|
||||||
|
* Serialize as Markdown for structured content:
|
||||||
|
* - lists with 2+ total items (a single copied bullet stays literal text);
|
||||||
|
* - a whole table (top-level `table` node);
|
||||||
|
* - a partial table cell-selection, which prosemirror-tables copies as bare
|
||||||
|
* `tableRow` nodes (only a full-table selection yields a `table` node).
|
||||||
|
*
|
||||||
|
* `wrapBareRows` flags the bare-rows case so the caller wraps the serialized
|
||||||
|
* <tr> nodes in <table><tbody> before the HTML->Markdown step. Plain paragraphs
|
||||||
|
* return asMarkdown=false so a simple text copy stays literal, and internal
|
||||||
|
* copy/paste keeps using the richer text/html clipboard payload.
|
||||||
|
*/
|
||||||
|
export function classifyClipboardSelection(
|
||||||
|
nodes: { name: string; childCount: number }[],
|
||||||
|
): { asMarkdown: boolean; wrapBareRows: boolean } {
|
||||||
|
const listTypes = ["bulletList", "orderedList", "taskList"];
|
||||||
|
let topLevelCount = 0;
|
||||||
|
let hasList = false;
|
||||||
|
let hasTable = false;
|
||||||
|
let tableRowCount = 0;
|
||||||
|
let nonRowCount = 0;
|
||||||
|
|
||||||
|
for (const node of nodes) {
|
||||||
|
if (listTypes.includes(node.name)) {
|
||||||
|
hasList = true;
|
||||||
|
topLevelCount += node.childCount;
|
||||||
|
nonRowCount++;
|
||||||
|
} else {
|
||||||
|
if (node.name === "table") hasTable = true;
|
||||||
|
if (node.name === "tableRow") tableRowCount++;
|
||||||
|
else nonRowCount++;
|
||||||
|
topLevelCount++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Bare tableRow nodes at the top level only occur for a partial cell
|
||||||
|
// selection; a slice never mixes bare rows with other block types, so
|
||||||
|
// "every top-level node is a row" is a safe signal to wrap-and-serialize.
|
||||||
|
const wrapBareRows = tableRowCount > 0 && nonRowCount === 0;
|
||||||
|
const asMarkdown =
|
||||||
|
(hasList && topLevelCount >= 2) || hasTable || wrapBareRows;
|
||||||
|
return { asMarkdown, wrapBareRows };
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Reorder/dedup the footnotes of a SELF-CONTAINED pasted markdown block to the
|
* Reorder/dedup the footnotes of a SELF-CONTAINED pasted markdown block to the
|
||||||
* canonical invariant (the live footnoteSyncPlugin never reorders an existing
|
* canonical invariant (the live footnoteSyncPlugin never reorders an existing
|
||||||
|
|||||||
@@ -100,7 +100,7 @@ describe("useScrollPosition", () => {
|
|||||||
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("(a3) restores at most once per mount even if called again", () => {
|
it("(a3) is idempotent: re-asserting the same target does not scroll again", () => {
|
||||||
vi.useFakeTimers();
|
vi.useFakeTimers();
|
||||||
window.sessionStorage.setItem(`${KEY_PREFIX}once`, "500");
|
window.sessionStorage.setItem(`${KEY_PREFIX}once`, "500");
|
||||||
setScrollHeight(2000); // tall enough to restore synchronously
|
setScrollHeight(2000); // tall enough to restore synchronously
|
||||||
@@ -111,8 +111,12 @@ describe("useScrollPosition", () => {
|
|||||||
});
|
});
|
||||||
expect(window.scrollTo).toHaveBeenCalledTimes(1);
|
expect(window.scrollTo).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Simulate the browser now being at the restored position.
|
||||||
|
setScrollY(500);
|
||||||
|
|
||||||
// A second call (e.g. the wiring effect re-running on [showStatic, editor,
|
// A second call (e.g. the wiring effect re-running on [showStatic, editor,
|
||||||
// restoreScrollPosition]) must NOT scroll again and yank the reader.
|
// restoreScrollPosition]) must NOT scroll again: the redundancy guard sees
|
||||||
|
// the window is already at the target and does nothing.
|
||||||
act(() => {
|
act(() => {
|
||||||
result.current.restoreScrollPosition();
|
result.current.restoreScrollPosition();
|
||||||
});
|
});
|
||||||
@@ -162,6 +166,84 @@ describe("useScrollPosition", () => {
|
|||||||
expect(window.scrollTo).not.toHaveBeenCalled();
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("(g) does not restore if the reader scrolled (wheel) before restore fires", () => {
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}g1`, "500");
|
||||||
|
setScrollHeight(2000); // tall enough to restore synchronously
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("g1"));
|
||||||
|
|
||||||
|
// The reader shows scroll intent before restore is triggered.
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event("wheel"));
|
||||||
|
});
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(h) aborts an in-flight restore poll when the reader scrolls", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}h1`, "500");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(100); // maxScroll = -700: target not reachable yet, so it polls.
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("h1"));
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
|
||||||
|
|
||||||
|
// The reader takes over mid-poll: this cancels the in-flight poll.
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event("wheel"));
|
||||||
|
});
|
||||||
|
|
||||||
|
// Content of the page grows tall enough and time passes: the cancelled poll
|
||||||
|
// must NOT resurrect and yank the reader.
|
||||||
|
setScrollHeight(2000);
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(5000);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(i) a non-scroll keydown does NOT abort restore", () => {
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}i1`, "500");
|
||||||
|
setScrollHeight(2000); // tall enough to restore synchronously
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("i1"));
|
||||||
|
|
||||||
|
// A non-scroll key (e.g. typing, a shortcut) must NOT count as scroll intent.
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
|
||||||
|
});
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Restore still happens: the innocuous keypress did not disable it.
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(j) a scroll keydown (Space) DOES abort restore", () => {
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}j1`, "500");
|
||||||
|
setScrollHeight(2000); // tall enough to restore synchronously
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("j1"));
|
||||||
|
|
||||||
|
// Space scrolls the page: this is real scroll intent and must abort restore.
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new KeyboardEvent("keydown", { key: " " }));
|
||||||
|
});
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
|
it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
|
||||||
// Nothing saved.
|
// Nothing saved.
|
||||||
const a = renderHook(() => useScrollPosition("nope"));
|
const a = renderHook(() => useScrollPosition("nope"));
|
||||||
@@ -221,6 +303,55 @@ describe("useScrollPosition", () => {
|
|||||||
expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("(k) shares ONE timeout budget across re-triggers (does not restart the clock)", () => {
|
||||||
|
// The static->live editor swap re-invokes restore. The shared budget
|
||||||
|
// (restoreStartRef) must measure the MAX_RESTORE_WAIT_MS (5000) deadline
|
||||||
|
// from the FIRST trigger, not restart it on every re-trigger. This pins
|
||||||
|
// the `if (restoreStartRef.current === null)` guard: a mutant that resets
|
||||||
|
// `restoreStartRef.current = Date.now()` on every trigger would push the
|
||||||
|
// deadline out to t=8000 (3000 + 5000) and fail the t=5000 assertion below.
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(0);
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}k1`, "5000");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(1000); // maxScroll = 200, never reaches 5000 -> it polls.
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("k1"));
|
||||||
|
|
||||||
|
// First trigger at t=0: starts the shared budget and begins polling.
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Advance to t=3000 (still polling: content short, not yet timed out).
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(3000);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Second trigger at t=3000 (the swap re-assert). Under the real code the
|
||||||
|
// budget is shared, so `start` stays 0; under the reset-mutant it becomes 3000.
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
// At t=4900 the FIRST budget has not yet elapsed (4900 - 0 < 5000): no clamp.
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(1900);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// At t=5000 the shared budget (measured from t=0) times out and clamps to the
|
||||||
|
// furthest reachable position (maxScroll = 200). The reset-mutant, measuring
|
||||||
|
// from t=3000, would still be waiting (5000 - 3000 = 2000 < 5000) and would
|
||||||
|
// NOT have scrolled here -> this assertion fails against that mutant.
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(100);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
|
||||||
|
});
|
||||||
|
|
||||||
it("(e) never throws when storage access throws", () => {
|
it("(e) never throws when storage access throws", () => {
|
||||||
const err = new Error("storage denied");
|
const err = new Error("storage denied");
|
||||||
vi.spyOn(window.sessionStorage, "getItem").mockImplementation(() => {
|
vi.spyOn(window.sessionStorage, "getItem").mockImplementation(() => {
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { useCallback, useEffect, useRef } from "react";
|
import { useCallback, useEffect, useLayoutEffect, useRef } from "react";
|
||||||
|
import type { Editor } from "@tiptap/react";
|
||||||
|
|
||||||
// Throttle interval for persisting the scroll position while the user reads.
|
// Throttle interval for persisting the scroll position while the user reads.
|
||||||
const SAVE_THROTTLE_MS = 250;
|
const SAVE_THROTTLE_MS = 250;
|
||||||
@@ -13,6 +14,18 @@ const RESTORE_POLL_MS = 100;
|
|||||||
// "remember where I was reading" feature (self-limiting, no cross-tab leak).
|
// "remember where I was reading" feature (self-limiting, no cross-tab leak).
|
||||||
const STORAGE_PREFIX = "gitmost:scroll-position:";
|
const STORAGE_PREFIX = "gitmost:scroll-position:";
|
||||||
|
|
||||||
|
// Keys that scroll the window. Only these count as scroll intent for keydown;
|
||||||
|
// other keys (shortcuts, modifiers, typing) must NOT disable scroll restore.
|
||||||
|
const SCROLL_KEYS = new Set([
|
||||||
|
"ArrowUp",
|
||||||
|
"ArrowDown",
|
||||||
|
"PageUp",
|
||||||
|
"PageDown",
|
||||||
|
"Home",
|
||||||
|
"End",
|
||||||
|
" ", // Space (and Shift+Space) scroll the page
|
||||||
|
]);
|
||||||
|
|
||||||
function storageKey(pageId: string): string {
|
function storageKey(pageId: string): string {
|
||||||
return `${STORAGE_PREFIX}${pageId}`;
|
return `${STORAGE_PREFIX}${pageId}`;
|
||||||
}
|
}
|
||||||
@@ -48,32 +61,41 @@ function writeStorage(pageId: string, scrollY: number): void {
|
|||||||
* Persists and restores the window scroll position per page so a reader keeps
|
* Persists and restores the window scroll position per page so a reader keeps
|
||||||
* their place across a reload (F5) or reopening the document.
|
* their place across a reload (F5) or reopening the document.
|
||||||
*
|
*
|
||||||
* Returns `restoreScrollPosition`, which the page editor calls once the live
|
* Returns `restoreScrollPosition`, which the page editor calls from two triggers
|
||||||
* (non-static) content is laid out. The two scroll mechanisms are mutually
|
* (early, while the static/cached content is laid out, and again after the
|
||||||
* exclusive: if the URL has a `#hash` anchor, the existing anchor-scroll logic
|
* static->live editor swap); it is idempotent, so re-asserting the same target is
|
||||||
* wins and restore is a no-op.
|
* a no-op. The two scroll mechanisms are mutually exclusive: if the URL has a
|
||||||
|
* `#hash` anchor, the existing anchor-scroll logic wins and restore is a no-op.
|
||||||
*/
|
*/
|
||||||
export function useScrollPosition(pageId: string): {
|
export function useScrollPosition(pageId: string): {
|
||||||
restoreScrollPosition: () => void;
|
restoreScrollPosition: () => void;
|
||||||
} {
|
} {
|
||||||
// CONTRACT: this hook assumes PageEditor REMOUNTS per page — page.tsx renders
|
// CONTRACT: this hook assumes PageEditor REMOUNTS per page — page.tsx renders
|
||||||
// `<MemoizedFullEditor key={page.id} ...>`, so switching pages creates a fresh
|
// `<MemoizedFullEditor key={page.id} ...>`, so switching pages creates a fresh
|
||||||
// hook instance with fresh refs. These refs latch per-mount and are NOT reset
|
// hook instance with fresh refs. Restore is idempotent and interaction-gated
|
||||||
// when `pageId` changes in place (only the effect re-runs on [pageId]). If that
|
// (not single-shot): it may be called from several triggers and re-asserts the
|
||||||
// `key={page.id}` is ever removed, restore would silently break on the 2nd page
|
// SAME captured target, which is a no-op once the window is already positioned.
|
||||||
// (refs would hold the first page's target / already-restored flag) — in that
|
// The per-mount refs that latch are `initialTargetRef` (the captured target)
|
||||||
// case the refs must be reset on a pageId change.
|
// and `userInteractedRef` (the reader has taken over scrolling). They are NOT
|
||||||
|
// reset when `pageId` changes in place (only the effect re-runs on [pageId]).
|
||||||
|
// If that `key={page.id}` is ever removed, restore would silently break on the
|
||||||
|
// 2nd page (refs would hold the first page's target / interaction flag) — in
|
||||||
|
// that case the refs must be reset on a pageId change.
|
||||||
//
|
//
|
||||||
// The target Y captured synchronously at mount, BEFORE any scroll/visibility
|
// The target Y captured synchronously at mount, BEFORE any scroll/visibility
|
||||||
// handler can overwrite the stored value with a fresh 0 (the page starts
|
// handler can overwrite the stored value with a fresh 0 (the page starts
|
||||||
// scrolled to top on load). `null` means "not yet captured".
|
// scrolled to top on load). `null` means "not yet captured".
|
||||||
const initialTargetRef = useRef<number | null>(null);
|
const initialTargetRef = useRef<number | null>(null);
|
||||||
// Guards so restore runs at most once per page mount.
|
// Set once the reader shows unambiguous scroll intent; restore must never yank
|
||||||
const hasRestoredRef = useRef(false);
|
// a reader who has already started scrolling.
|
||||||
|
const userInteractedRef = useRef(false);
|
||||||
// Holds the in-flight restore poll timer so the cleanup can cancel it: without
|
// Holds the in-flight restore poll timer so the cleanup can cancel it: without
|
||||||
// this, a fast SPA navigation away mid-poll would let the old page's poll fire
|
// this, a fast SPA navigation away mid-poll would let the old page's poll fire
|
||||||
// window.scrollTo against the NEW page's document (visible wrong-page scroll).
|
// window.scrollTo against the NEW page's document (visible wrong-page scroll).
|
||||||
const pollTimerRef = useRef<number | null>(null);
|
const pollTimerRef = useRef<number | null>(null);
|
||||||
|
// Timestamp of the FIRST restore attempt so re-triggers (e.g. the static→live
|
||||||
|
// editor swap) share ONE bounded timeout budget instead of restarting it.
|
||||||
|
const restoreStartRef = useRef<number | null>(null);
|
||||||
|
|
||||||
// Capture the previously-saved value synchronously during render, before the
|
// Capture the previously-saved value synchronously during render, before the
|
||||||
// effect below registers handlers that would persist the current (0) scrollY.
|
// effect below registers handlers that would persist the current (0) scrollY.
|
||||||
@@ -114,14 +136,43 @@ export function useScrollPosition(pageId: string): {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// User scroll-intent signals. wheel and touch are unconditional scroll
|
||||||
|
// intent; keydown is filtered to actual scroll keys only (SCROLL_KEYS) so
|
||||||
|
// shortcuts, lone modifiers, and typing do not abort restore. Our own
|
||||||
|
// window.scrollTo does NOT emit these, so restore can never self-abort via
|
||||||
|
// them. Once the reader shows intent we mark it and cancel any in-flight
|
||||||
|
// restore poll so restore can never yank them back. (Scrollbar-drag via
|
||||||
|
// pointer is an accepted small gap — it is not covered here.)
|
||||||
|
const onUserIntent = (event: Event) => {
|
||||||
|
// wheel/touchstart are unambiguous scroll intent; for keydown, only real
|
||||||
|
// scroll keys count — a shortcut or typing must not abort restore.
|
||||||
|
if (
|
||||||
|
event.type === "keydown" &&
|
||||||
|
!SCROLL_KEYS.has((event as KeyboardEvent).key)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
userInteractedRef.current = true;
|
||||||
|
if (pollTimerRef.current !== null) {
|
||||||
|
window.clearTimeout(pollTimerRef.current);
|
||||||
|
pollTimerRef.current = null;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
window.addEventListener("scroll", onScroll, { passive: true });
|
window.addEventListener("scroll", onScroll, { passive: true });
|
||||||
window.addEventListener("pagehide", onPageHide);
|
window.addEventListener("pagehide", onPageHide);
|
||||||
document.addEventListener("visibilitychange", onVisibilityChange);
|
document.addEventListener("visibilitychange", onVisibilityChange);
|
||||||
|
window.addEventListener("wheel", onUserIntent, { passive: true });
|
||||||
|
window.addEventListener("touchstart", onUserIntent, { passive: true });
|
||||||
|
window.addEventListener("keydown", onUserIntent);
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
window.removeEventListener("scroll", onScroll);
|
window.removeEventListener("scroll", onScroll);
|
||||||
window.removeEventListener("pagehide", onPageHide);
|
window.removeEventListener("pagehide", onPageHide);
|
||||||
document.removeEventListener("visibilitychange", onVisibilityChange);
|
document.removeEventListener("visibilitychange", onVisibilityChange);
|
||||||
|
window.removeEventListener("wheel", onUserIntent);
|
||||||
|
window.removeEventListener("touchstart", onUserIntent);
|
||||||
|
window.removeEventListener("keydown", onUserIntent);
|
||||||
if (throttleTimer !== null) {
|
if (throttleTimer !== null) {
|
||||||
window.clearTimeout(throttleTimer);
|
window.clearTimeout(throttleTimer);
|
||||||
throttleTimer = null;
|
throttleTimer = null;
|
||||||
@@ -137,9 +188,8 @@ export function useScrollPosition(pageId: string): {
|
|||||||
}, [pageId]);
|
}, [pageId]);
|
||||||
|
|
||||||
const restoreScrollPosition = useCallback(() => {
|
const restoreScrollPosition = useCallback(() => {
|
||||||
// Run at most once per page mount.
|
// The reader took over — never yank them back.
|
||||||
if (hasRestoredRef.current) return;
|
if (userInteractedRef.current) return;
|
||||||
hasRestoredRef.current = true;
|
|
||||||
|
|
||||||
// Anchor priority: a `#hash` in the URL is handled by useEditorScroll.
|
// Anchor priority: a `#hash` in the URL is handled by useEditorScroll.
|
||||||
if (window.location.hash) return;
|
if (window.location.hash) return;
|
||||||
@@ -148,9 +198,26 @@ export function useScrollPosition(pageId: string): {
|
|||||||
// Nothing meaningful to restore to.
|
// Nothing meaningful to restore to.
|
||||||
if (targetY <= 0) return;
|
if (targetY <= 0) return;
|
||||||
|
|
||||||
const start = Date.now();
|
// Cancel any in-flight poll before (re)starting, so overlapping triggers can
|
||||||
|
// never run two concurrent polls against the same target.
|
||||||
|
if (pollTimerRef.current !== null) {
|
||||||
|
window.clearTimeout(pollTimerRef.current);
|
||||||
|
pollTimerRef.current = null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Share one timeout budget across re-triggers instead of restarting it.
|
||||||
|
if (restoreStartRef.current === null) {
|
||||||
|
restoreStartRef.current = Date.now();
|
||||||
|
}
|
||||||
|
const start = restoreStartRef.current;
|
||||||
|
|
||||||
const tryRestore = () => {
|
const tryRestore = () => {
|
||||||
|
// Bail mid-poll if the reader started scrolling while we were waiting.
|
||||||
|
if (userInteractedRef.current) {
|
||||||
|
pollTimerRef.current = null;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const maxScroll =
|
const maxScroll =
|
||||||
document.documentElement.scrollHeight - window.innerHeight;
|
document.documentElement.scrollHeight - window.innerHeight;
|
||||||
const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
|
const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
|
||||||
@@ -158,10 +225,12 @@ export function useScrollPosition(pageId: string): {
|
|||||||
// Restore once the content is tall enough to reach the target, or bail out
|
// Restore once the content is tall enough to reach the target, or bail out
|
||||||
// after the timeout and scroll as far as currently possible.
|
// after the timeout and scroll as far as currently possible.
|
||||||
if (maxScroll >= targetY || timedOut) {
|
if (maxScroll >= targetY || timedOut) {
|
||||||
window.scrollTo({
|
const top = Math.min(targetY, Math.max(maxScroll, 0));
|
||||||
top: Math.min(targetY, Math.max(maxScroll, 0)),
|
// Redundancy guard: re-asserting the SAME target when already positioned
|
||||||
behavior: "auto",
|
// is a no-op, so this hook can be called from multiple triggers safely.
|
||||||
});
|
if (Math.abs(window.scrollY - top) > 1) {
|
||||||
|
window.scrollTo({ top, behavior: "auto" });
|
||||||
|
}
|
||||||
pollTimerRef.current = null;
|
pollTimerRef.current = null;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -175,3 +244,37 @@ export function useScrollPosition(pageId: string): {
|
|||||||
|
|
||||||
return { restoreScrollPosition };
|
return { restoreScrollPosition };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Wires `useScrollPosition` to the page editor's static->live swap lifecycle.
|
||||||
|
*
|
||||||
|
* Extracted from PageEditor so the exact restore triggers (their deps and the
|
||||||
|
* post-swap `&& editor` guard) are directly unit-testable rather than mirrored.
|
||||||
|
* Behaviour is unchanged: `restoreScrollPosition` is idempotent, so re-asserting
|
||||||
|
* the same target from either trigger is a no-op.
|
||||||
|
*
|
||||||
|
* @param pageId the page whose scroll position is persisted/restored.
|
||||||
|
* @param editor the tiptap editor instance, or `null` until it is ready.
|
||||||
|
* @param showStatic whether the static (cached) content is still shown.
|
||||||
|
*/
|
||||||
|
export function useScrollRestoreOnSwap(
|
||||||
|
pageId: string,
|
||||||
|
editor: Editor | null,
|
||||||
|
showStatic: boolean,
|
||||||
|
): void {
|
||||||
|
const { restoreScrollPosition } = useScrollPosition(pageId);
|
||||||
|
|
||||||
|
// Restore as early as the static (cached) content is laid out, before paint,
|
||||||
|
// so the reader's position is applied without a visible jump. Aborts itself if
|
||||||
|
// the reader has already started scrolling (handled inside the hook).
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
restoreScrollPosition();
|
||||||
|
}, [restoreScrollPosition]);
|
||||||
|
|
||||||
|
// Re-assert once after the static -> live editor swap in case the swap reset
|
||||||
|
// the window scroll. Idempotent: a no-op when the position is already correct,
|
||||||
|
// and a no-op after the reader has interacted.
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
if (!showStatic && editor) restoreScrollPosition();
|
||||||
|
}, [showStatic, editor, restoreScrollPosition]);
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,141 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { render, act } from "@testing-library/react";
|
||||||
|
import type { Editor } from "@tiptap/react";
|
||||||
|
import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
|
||||||
|
|
||||||
|
const KEY_PREFIX = "gitmost:scroll-position:";
|
||||||
|
|
||||||
|
// NOTE ON SCOPE (F2 — reviewer-approved lighter variant).
|
||||||
|
//
|
||||||
|
// The real UX wiring lives in the exported `useScrollRestoreOnSwap` hook (two
|
||||||
|
// useLayoutEffects around useScrollPosition), which PageEditor calls with the
|
||||||
|
// same signature. A FULL PageEditor component test is impractical here and has no
|
||||||
|
// precedent in this client: PageEditor directly constructs a
|
||||||
|
// HocuspocusProviderWebsocket + IndexeddbPersistence, a tiptap `useEditor` with
|
||||||
|
// collab extensions, reads jotai atoms, react-router params, the shared
|
||||||
|
// `queryClient` from main.tsx, i18n, and mounts ~12 editor menu children. Worse,
|
||||||
|
// the static->live swap (`showStatic` -> false) is gated on
|
||||||
|
// `isCollabSynced(status, isLocalSynced && isRemoteSynced)`, which can only flip
|
||||||
|
// by driving the mocked collab provider's async sync callbacks. The heaviest
|
||||||
|
// component-test precedent in the repo (comment-hover-preview.test.tsx) mounts a
|
||||||
|
// single leaf component with ONE mocked query; nothing mounts a feature root of
|
||||||
|
// this weight. Reproducing all of that would test the mocks, not the wiring.
|
||||||
|
//
|
||||||
|
// So this file tests the REAL `useScrollRestoreOnSwap` hook — the exact code
|
||||||
|
// PageEditor imports and calls — driving its `showStatic`/`editor` inputs the way
|
||||||
|
// the swap does. Because it exercises the real hook (not a copy), dropping the
|
||||||
|
// `&& editor` guard or changing the effect deps makes these tests fail; they
|
||||||
|
// guard the production code directly (verified: removing `&& editor` reddens the
|
||||||
|
// first test).
|
||||||
|
//
|
||||||
|
// Both tests observe the real effect via `window.scrollTo`. The stubbed
|
||||||
|
// `window.scrollTo` never mutates `window.scrollY`, and the target is left
|
||||||
|
// unreached, so every restore invocation that passes the guard yields exactly one
|
||||||
|
// `scrollTo` call — making the call count a faithful proxy for restore invocations.
|
||||||
|
|
||||||
|
function setScrollY(value: number): void {
|
||||||
|
Object.defineProperty(window, "scrollY", { configurable: true, value });
|
||||||
|
}
|
||||||
|
function setScrollHeight(value: number): void {
|
||||||
|
Object.defineProperty(document.documentElement, "scrollHeight", {
|
||||||
|
configurable: true,
|
||||||
|
value,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
function setInnerHeight(value: number): void {
|
||||||
|
Object.defineProperty(window, "innerHeight", { configurable: true, value });
|
||||||
|
}
|
||||||
|
|
||||||
|
// Minimal stand-in for the tiptap editor: the hook only truthiness-checks it.
|
||||||
|
const fakeEditor = { id: "editor" } as unknown as Editor;
|
||||||
|
|
||||||
|
// Thin host that calls the REAL hook so a rerender drives showStatic/editor
|
||||||
|
// exactly like the page-editor swap does.
|
||||||
|
function Host({
|
||||||
|
pageId,
|
||||||
|
showStatic,
|
||||||
|
editor,
|
||||||
|
}: {
|
||||||
|
pageId: string;
|
||||||
|
showStatic: boolean;
|
||||||
|
editor: Editor | null;
|
||||||
|
}) {
|
||||||
|
useScrollRestoreOnSwap(pageId, editor, showStatic);
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("PageEditor scroll-restore wiring (useScrollRestoreOnSwap)", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
window.sessionStorage.clear();
|
||||||
|
setScrollY(0);
|
||||||
|
setScrollHeight(0);
|
||||||
|
setInnerHeight(800);
|
||||||
|
window.scrollTo = vi.fn();
|
||||||
|
window.location.hash = "";
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
vi.useRealTimers();
|
||||||
|
window.location.hash = "";
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-invokes restore after the swap, with the [showStatic, editor] deps/guard", () => {
|
||||||
|
// Target is immediately reachable, so each restore that passes the guard
|
||||||
|
// scrolls synchronously. `window.scrollY` stays 0 (stubbed scrollTo never
|
||||||
|
// updates it), so scrollTo is called once per effective restore — a proxy for
|
||||||
|
// the restore invocation count.
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}guard`, "500");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(2000); // maxScroll = 1200 >= 500: reachable, no polling.
|
||||||
|
|
||||||
|
// Pre-swap: static content shown, live editor not ready. Only the early
|
||||||
|
// pre-paint restore fires; the post-swap effect's guard (!showStatic) blocks it.
|
||||||
|
const { rerender } = render(
|
||||||
|
<Host pageId="guard" showStatic={true} editor={null} />,
|
||||||
|
);
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Collab reports synced (showStatic flips false) but the editor is not ready
|
||||||
|
// yet: the swap effect re-runs (deps [showStatic, editor] changed) but the
|
||||||
|
// `&& editor` guard must keep it a no-op. The early effect does NOT re-fire
|
||||||
|
// (its dep [restoreScrollPosition] is a stable useCallback([])).
|
||||||
|
// (Pins the guard: dropping `&& editor` would restore against a null editor,
|
||||||
|
// producing a 2nd scrollTo and failing this expectation.)
|
||||||
|
rerender(<Host pageId="guard" showStatic={false} editor={null} />);
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// The static -> live swap completes (showStatic false AND editor present): the
|
||||||
|
// post-swap effect re-asserts the restore exactly once more, driven solely by
|
||||||
|
// the [showStatic, editor] deps changing.
|
||||||
|
rerender(<Host pageId="guard" showStatic={false} editor={fakeEditor} />);
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("the post-swap re-assert drives a REAL restore (window.scrollTo) via the hook", () => {
|
||||||
|
// End-to-end through the real useScrollPosition (inside the hook): the swap
|
||||||
|
// re-invocation is the CAUSE of the scroll (nothing scrolls before it).
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}peg`, "500");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(100); // maxScroll = -700: target not reachable yet -> polls.
|
||||||
|
|
||||||
|
// Pre-swap: the early restore runs but content is too short, so it starts
|
||||||
|
// polling (a pending timer) without scrolling. We never advance timers, so the
|
||||||
|
// early poll cannot fire on its own — isolating the swap as the sole cause.
|
||||||
|
const { rerender } = render(
|
||||||
|
<Host pageId="peg" showStatic={true} editor={null} />,
|
||||||
|
);
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// The live content is now laid out tall enough to reach the target.
|
||||||
|
setScrollHeight(2000); // maxScroll = 1200 >= 500
|
||||||
|
|
||||||
|
// The static -> live swap: the post-swap useLayoutEffect re-invokes the real
|
||||||
|
// hook, whose synchronous tryRestore now reaches the target and scrolls.
|
||||||
|
act(() => {
|
||||||
|
rerender(<Host pageId="peg" showStatic={false} editor={fakeEditor} />);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -78,7 +78,7 @@ import { PageEditMode } from "@/features/user/types/user.types.ts";
|
|||||||
import { jwtDecode } from "jwt-decode";
|
import { jwtDecode } from "jwt-decode";
|
||||||
import { searchSpotlight } from "@/features/search/constants.ts";
|
import { searchSpotlight } from "@/features/search/constants.ts";
|
||||||
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
||||||
import { useScrollPosition } from "./hooks/use-scroll-position";
|
import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
|
||||||
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
||||||
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
||||||
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
||||||
@@ -143,7 +143,6 @@ export default function PageEditor({
|
|||||||
[isComponentMounted],
|
[isComponentMounted],
|
||||||
);
|
);
|
||||||
const { handleScrollTo } = useEditorScroll({ canScroll });
|
const { handleScrollTo } = useEditorScroll({ canScroll });
|
||||||
const { restoreScrollPosition } = useScrollPosition(pageId);
|
|
||||||
// Providers only created once per pageId
|
// Providers only created once per pageId
|
||||||
const providersRef = useRef<{
|
const providersRef = useRef<{
|
||||||
local: IndexeddbPersistence;
|
local: IndexeddbPersistence;
|
||||||
@@ -482,10 +481,10 @@ export default function PageEditor({
|
|||||||
}
|
}
|
||||||
}, [yjsConnectionStatus, isSynced]);
|
}, [yjsConnectionStatus, isSynced]);
|
||||||
|
|
||||||
// Restore the saved reading position once the live content is laid out.
|
// Restore the reader's scroll position across the static -> live editor swap.
|
||||||
useEffect(() => {
|
// The wiring (early pre-paint restore + post-swap re-assert) lives in the hook
|
||||||
if (!showStatic && editor) restoreScrollPosition();
|
// so its triggers/guard are directly unit-testable.
|
||||||
}, [showStatic, editor, restoreScrollPosition]);
|
useScrollRestoreOnSwap(pageId, editor, showStatic);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<TransclusionLookupProvider>
|
<TransclusionLookupProvider>
|
||||||
|
|||||||
@@ -71,3 +71,22 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Inline image rows (#284): center the anonymous line boxes formed by
|
||||||
|
consecutive [data-image-align="inline"] node-view containers. A row has no
|
||||||
|
DOM wrapper of its own, so its horizontal placement is controlled by the
|
||||||
|
text-align of the nearest block ancestor (the editor root or a nested
|
||||||
|
block container: blockquote, callout, list item, table cell, details).
|
||||||
|
Centering is enabled only in containers that actually hold an inline
|
||||||
|
image (:has), and every other child of such a container gets its default
|
||||||
|
alignment back so ordinary text is unaffected. Explicit per-block
|
||||||
|
alignment from the toolbar is an inline style and still wins. Browsers
|
||||||
|
without :has() degrade to left-pinned rows. */
|
||||||
|
.ProseMirror:has(> [data-image-align="inline"]),
|
||||||
|
.ProseMirror :has(> [data-image-align="inline"]) {
|
||||||
|
text-align: center;
|
||||||
|
}
|
||||||
|
|
||||||
|
.ProseMirror:has(> [data-image-align="inline"]) > :not([data-image-align="inline"]),
|
||||||
|
.ProseMirror :has(> [data-image-align="inline"]) > :not([data-image-align="inline"]) {
|
||||||
|
text-align: start;
|
||||||
|
}
|
||||||
|
|||||||
@@ -13,20 +13,30 @@ export type OpenMap = Record<string, boolean>;
|
|||||||
// `OpenMap | Promise<OpenMap>` and break the functional-updater setter below).
|
// `OpenMap | Promise<OpenMap>` and break the functional-updater setter below).
|
||||||
const openTreeNodesStorage = createJSONStorage<OpenMap>(() => localStorage);
|
const openTreeNodesStorage = createJSONStorage<OpenMap>(() => localStorage);
|
||||||
|
|
||||||
|
// Single source of truth for the open-map localStorage key prefix. Exported so
|
||||||
|
// the logout cache sweep (tree-data-atom.ts) removes keys by the SAME prefix
|
||||||
|
// used to write them — a rename here can never silently desync the cleanup.
|
||||||
|
export const OPEN_TREE_NODES_KEY_PREFIX = "openTreeNodes:";
|
||||||
|
|
||||||
// One persisted open/closed map per (workspace, user). Scoping the localStorage
|
// One persisted open/closed map per (workspace, user). Scoping the localStorage
|
||||||
// key prevents accounts that share a browser origin from leaking tree state.
|
// key prevents accounts that share a browser origin from leaking tree state.
|
||||||
// `getOnInit: true` reads localStorage synchronously at atom init (not on mount),
|
// `getOnInit: true` reads localStorage synchronously at atom init (not on mount),
|
||||||
// so the first render already has the saved state — no collapse-then-expand
|
// so the first render already has the saved state — no collapse-then-expand
|
||||||
// flicker on reload, and writes never run against an un-hydrated empty map.
|
// flicker on reload, and writes never run against an un-hydrated empty map.
|
||||||
const openTreeNodesFamily = atomFamily((scopeKey: string) =>
|
const openTreeNodesFamily = atomFamily((scopeKey: string) =>
|
||||||
atomWithStorage<OpenMap>(`openTreeNodes:${scopeKey}`, {}, openTreeNodesStorage, {
|
atomWithStorage<OpenMap>(
|
||||||
getOnInit: true,
|
`${OPEN_TREE_NODES_KEY_PREFIX}${scopeKey}`,
|
||||||
}),
|
{},
|
||||||
|
openTreeNodesStorage,
|
||||||
|
{ getOnInit: true },
|
||||||
|
),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Resolve the storage scope from the current user. Fall back to "anon" for the
|
// Resolve the storage scope from the current user. Fall back to "anon" for the
|
||||||
// workspace/user parts when nothing is loaded yet (logged out / first paint).
|
// workspace/user parts when nothing is loaded yet (logged out / first paint).
|
||||||
const scopeKeyAtom = atom((get) => {
|
// Shared by the open-map atom below and the persisted tree-data atom
|
||||||
|
// (tree-data-atom.ts) so both caches are scoped identically.
|
||||||
|
export const scopeKeyAtom = atom((get) => {
|
||||||
const currentUser = get(currentUserAtom);
|
const currentUser = get(currentUserAtom);
|
||||||
const workspaceId = currentUser?.workspace?.id ?? "anon";
|
const workspaceId = currentUser?.workspace?.id ?? "anon";
|
||||||
const userId = currentUser?.user?.id ?? "anon";
|
const userId = currentUser?.user?.id ?? "anon";
|
||||||
|
|||||||
@@ -0,0 +1,265 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import type { SpaceTreeNode } from "@/features/page/tree/types";
|
||||||
|
import type { ICurrentUser } from "@/features/user/types/user.types";
|
||||||
|
|
||||||
|
// The persisted tree-data atom hydrates from localStorage ONCE, at family-atom
|
||||||
|
// creation (`getOnInit: true`). To exercise hydration deterministically each
|
||||||
|
// test imports a FRESH module instance (fresh atomFamily) after seeding the
|
||||||
|
// storage stub from vitest.setup.ts. jotai itself is externalized by vitest, so
|
||||||
|
// `createStore` can stay a static import — atoms are plain objects and any
|
||||||
|
// store works with any module instance.
|
||||||
|
import { createStore } from "jotai";
|
||||||
|
|
||||||
|
// Storage key for the default scope: no currentUser -> "anon:anon" (see
|
||||||
|
// scopeKeyAtom in open-tree-nodes-atom.ts) with the `v1` cache-shape version.
|
||||||
|
const ANON_KEY = "treeData:v1:anon:anon";
|
||||||
|
const DEBOUNCE_MS = 500;
|
||||||
|
|
||||||
|
async function freshImport() {
|
||||||
|
vi.resetModules();
|
||||||
|
const treeDataModule = await import("./tree-data-atom");
|
||||||
|
const userModule = await import(
|
||||||
|
"@/features/user/atoms/current-user-atom"
|
||||||
|
);
|
||||||
|
return {
|
||||||
|
treeDataAtom: treeDataModule.treeDataAtom,
|
||||||
|
flushPendingTreeDataWrites: treeDataModule.flushPendingTreeDataWrites,
|
||||||
|
clearPersistedTreeCaches: treeDataModule.clearPersistedTreeCaches,
|
||||||
|
currentUserAtom: userModule.currentUserAtom,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function node(id: string): SpaceTreeNode {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
slugId: `slug-${id}`,
|
||||||
|
name: id,
|
||||||
|
position: "a0",
|
||||||
|
spaceId: "space-1",
|
||||||
|
parentPageId: null as unknown as string,
|
||||||
|
hasChildren: false,
|
||||||
|
children: [],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// Every persisted tree key currently in storage — asserting on the whole
|
||||||
|
// prefix (not one known key) catches writes that resurrect under ANY scope.
|
||||||
|
function persistedTreeDataKeys(): string[] {
|
||||||
|
const keys: string[] = [];
|
||||||
|
for (let i = 0; i < localStorage.length; i++) {
|
||||||
|
const key = localStorage.key(i);
|
||||||
|
if (key !== null && key.startsWith("treeData:v1:")) keys.push(key);
|
||||||
|
}
|
||||||
|
return keys;
|
||||||
|
}
|
||||||
|
|
||||||
|
function currentUser(workspaceId: string, userId: string): ICurrentUser {
|
||||||
|
return {
|
||||||
|
user: { id: userId },
|
||||||
|
workspace: { id: workspaceId },
|
||||||
|
} as unknown as ICurrentUser;
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
localStorage.clear();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("treeDataAtom (localStorage-persisted)", () => {
|
||||||
|
it("reads [] from a fresh store with empty storage", async () => {
|
||||||
|
const { treeDataAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
expect(store.get(treeDataAtom)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("persists through the debounced setItem and hydrates a fresh module back", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const setItemSpy = vi.spyOn(localStorage, "setItem");
|
||||||
|
|
||||||
|
const { treeDataAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
store.set(treeDataAtom, [node("a")]);
|
||||||
|
// Second write inside the debounce window — must coalesce into ONE flush
|
||||||
|
// carrying only the latest value.
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS / 2);
|
||||||
|
store.set(treeDataAtom, [node("a"), node("b")]);
|
||||||
|
|
||||||
|
// Nothing flushed yet: the write is trailing-debounced.
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS + 100);
|
||||||
|
|
||||||
|
expect(setItemSpy).toHaveBeenCalledTimes(1);
|
||||||
|
expect(JSON.parse(localStorage.getItem(ANON_KEY)!)).toEqual([
|
||||||
|
node("a"),
|
||||||
|
node("b"),
|
||||||
|
]);
|
||||||
|
|
||||||
|
// A fresh module (fresh atom family -> getOnInit re-reads storage) and a
|
||||||
|
// fresh store hydrate the persisted tree back — the reload scenario.
|
||||||
|
const second = await freshImport();
|
||||||
|
const store2 = createStore();
|
||||||
|
expect(store2.get(second.treeDataAtom)).toEqual([node("a"), node("b")]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reads [] (without throwing) when storage holds corrupted JSON", async () => {
|
||||||
|
localStorage.setItem(ANON_KEY, "{definitely not JSON!!!");
|
||||||
|
|
||||||
|
const { treeDataAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
expect(store.get(treeDataAtom)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reads [] when storage holds valid JSON of a non-array shape", async () => {
|
||||||
|
localStorage.setItem(ANON_KEY, JSON.stringify({ id: "not-a-tree" }));
|
||||||
|
|
||||||
|
const { treeDataAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
expect(store.get(treeDataAtom)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("supports functional-updater writes", async () => {
|
||||||
|
const { treeDataAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
store.set(treeDataAtom, [node("a")]);
|
||||||
|
store.set(treeDataAtom, (prev) => [...prev, node("b")]);
|
||||||
|
|
||||||
|
expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a", "b"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("isolates trees between (workspace, user) scopes", async () => {
|
||||||
|
const { treeDataAtom, currentUserAtom } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
store.set(currentUserAtom, currentUser("w1", "u1"));
|
||||||
|
store.set(treeDataAtom, [node("a")]);
|
||||||
|
expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a"]);
|
||||||
|
|
||||||
|
// Another account on the same browser origin must NOT see u1's tree.
|
||||||
|
store.set(currentUserAtom, currentUser("w2", "u2"));
|
||||||
|
expect(store.get(treeDataAtom)).toEqual([]);
|
||||||
|
|
||||||
|
store.set(treeDataAtom, [node("b")]);
|
||||||
|
expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["b"]);
|
||||||
|
|
||||||
|
// Switching back resolves the original scope's tree untouched.
|
||||||
|
store.set(currentUserAtom, currentUser("w1", "u1"));
|
||||||
|
expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clearPersistedTreeCaches removes all tree keys and discards pending writes", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
// Stale caches across scopes plus an UNRELATED key that must survive.
|
||||||
|
localStorage.setItem("treeData:v1:a:b", JSON.stringify([node("stale")]));
|
||||||
|
localStorage.setItem("openTreeNodes:a:b", JSON.stringify({ p1: true }));
|
||||||
|
localStorage.setItem("currentUser", JSON.stringify({ user: { id: "b" } }));
|
||||||
|
|
||||||
|
const { treeDataAtom, clearPersistedTreeCaches } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
// Queue a debounced write (not flushed yet) for the anon scope.
|
||||||
|
store.set(treeDataAtom, [node("pending")]);
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
|
||||||
|
clearPersistedTreeCaches();
|
||||||
|
|
||||||
|
// Both prefixed caches are swept; the unrelated key is untouched.
|
||||||
|
expect(localStorage.getItem("treeData:v1:a:b")).toBeNull();
|
||||||
|
expect(localStorage.getItem("openTreeNodes:a:b")).toBeNull();
|
||||||
|
expect(localStorage.getItem("currentUser")).toBe(
|
||||||
|
JSON.stringify({ user: { id: "b" } }),
|
||||||
|
);
|
||||||
|
|
||||||
|
// The queued write was DISCARDED, not merely delayed: the debounce timer
|
||||||
|
// firing later must not resurrect a tree key after logout.
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS + 100);
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clearPersistedTreeCaches discards queued writes even when flushed DIRECTLY", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
const { treeDataAtom, clearPersistedTreeCaches, flushPendingTreeDataWrites } =
|
||||||
|
await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
// Queue a debounced write, then clear. Calling the flush directly (not via
|
||||||
|
// the debounce timer) isolates the pending-queue discard from the timer
|
||||||
|
// cancel: if the queue survived, this flush would resurrect the key even
|
||||||
|
// though the timer never fired.
|
||||||
|
store.set(treeDataAtom, [node("pending")]);
|
||||||
|
clearPersistedTreeCaches();
|
||||||
|
flushPendingTreeDataWrites();
|
||||||
|
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
expect(persistedTreeDataKeys()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips persisting a tree over the size cap and warns exactly once", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
|
const setItemSpy = vi.spyOn(localStorage, "setItem");
|
||||||
|
|
||||||
|
const { treeDataAtom, flushPendingTreeDataWrites } = await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
// One node whose name alone serializes to > MAX_SERIALIZED_LENGTH (~4M).
|
||||||
|
const huge = node("big");
|
||||||
|
huge.name = "x".repeat(4_000_001);
|
||||||
|
|
||||||
|
store.set(treeDataAtom, [huge]);
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS + 100);
|
||||||
|
|
||||||
|
// The oversized serialization is skipped: the key is never written.
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
expect(setItemSpy).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Editing the still-oversized tree fires another debounced write, but the
|
||||||
|
// "too large" warn is gated by the once-flag — no per-tick console spam.
|
||||||
|
store.set(treeDataAtom, [huge, node("big2")]);
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS + 100);
|
||||||
|
flushPendingTreeDataWrites();
|
||||||
|
|
||||||
|
expect(localStorage.getItem(ANON_KEY)).toBeNull();
|
||||||
|
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||||
|
expect(warnSpy).toHaveBeenCalledWith(
|
||||||
|
"[tree] cached tree too large to persist; skipping",
|
||||||
|
ANON_KEY,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("disables persistence after clearPersistedTreeCaches: NEW writes never reach storage", async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
|
||||||
|
const { treeDataAtom, clearPersistedTreeCaches, flushPendingTreeDataWrites } =
|
||||||
|
await freshImport();
|
||||||
|
const store = createStore();
|
||||||
|
|
||||||
|
clearPersistedTreeCaches();
|
||||||
|
|
||||||
|
// The resurrection scenario: a websocket tree event lands while `await
|
||||||
|
// logout()` is still in flight, AFTER the sweep. The write must not be
|
||||||
|
// queued, must not arm a new debounce timer, and must not survive the
|
||||||
|
// beforeunload flush fired by the logout redirect.
|
||||||
|
store.set(treeDataAtom, [node("late")]);
|
||||||
|
|
||||||
|
vi.advanceTimersByTime(DEBOUNCE_MS + 100);
|
||||||
|
flushPendingTreeDataWrites(); // what the beforeunload handler runs
|
||||||
|
|
||||||
|
expect(persistedTreeDataKeys()).toEqual([]);
|
||||||
|
|
||||||
|
// Only PERSISTENCE is disabled: the in-memory atom keeps working, so the
|
||||||
|
// UI stays intact during the brief pre-redirect window.
|
||||||
|
expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["late"]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,8 +1,206 @@
|
|||||||
import { atom } from "jotai";
|
import { atom } from "jotai";
|
||||||
|
import { atomFamily, atomWithStorage } from "jotai/utils";
|
||||||
import { SpaceTreeNode } from "@/features/page/tree/types";
|
import { SpaceTreeNode } from "@/features/page/tree/types";
|
||||||
import { appendNodeChildren } from "../utils";
|
import { appendNodeChildren } from "../utils";
|
||||||
|
import {
|
||||||
|
OPEN_TREE_NODES_KEY_PREFIX,
|
||||||
|
scopeKeyAtom,
|
||||||
|
} from "./open-tree-nodes-atom";
|
||||||
|
|
||||||
export const treeDataAtom = atom<SpaceTreeNode[]>([]);
|
// The sidebar tree is persisted to localStorage so a page reload can paint the
|
||||||
|
// last-known tree IMMEDIATELY (no blank sidebar while the root query runs) and
|
||||||
|
// then reconcile with the server in the background. localStorage is a BOOT
|
||||||
|
// CACHE only — the in-memory atom stays the source of truth while the app runs.
|
||||||
|
|
||||||
|
// Trailing-debounce machinery for the localStorage writes. The tree is
|
||||||
|
// rewritten on every lazy load / drag / socket event; serializing a large tree
|
||||||
|
// on each update would burn CPU and thrash the storage quota, so writes are
|
||||||
|
// coalesced (~500 ms per burst) and only the latest value per key is flushed.
|
||||||
|
const WRITE_DEBOUNCE_MS = 500;
|
||||||
|
|
||||||
|
// Single source of truth for the tree-cache localStorage key prefix. The `v1`
|
||||||
|
// segment versions the cached node shape (bump it when SpaceTreeNode changes
|
||||||
|
// incompatibly). Shared by the storage key construction below AND the logout
|
||||||
|
// sweep in clearPersistedTreeCaches() so the two can never drift apart.
|
||||||
|
export const TREE_DATA_KEY_PREFIX = "treeData:v1:";
|
||||||
|
|
||||||
|
// Size guard: skip persisting trees whose JSON exceeds ~4M chars. localStorage
|
||||||
|
// quota is typically ~5 MB per origin; a huge tree must not evict everything
|
||||||
|
// else or spam QuotaExceededError on every debounce tick.
|
||||||
|
const MAX_SERIALIZED_LENGTH = 4_000_000;
|
||||||
|
|
||||||
|
const pendingWrites = new Map<string, SpaceTreeNode[]>();
|
||||||
|
let flushTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
|
let writeFailureWarned = false;
|
||||||
|
|
||||||
|
// Persistence kill-switch, armed by clearPersistedTreeCaches(). Once set, the
|
||||||
|
// debounced setItem and the flush become no-ops so nothing can be written back
|
||||||
|
// to localStorage AFTER the logout sweep: a websocket tree event landing while
|
||||||
|
// `await logout()` is still in flight would otherwise re-queue a write that
|
||||||
|
// the `beforeunload` flush (fired by the redirect) silently resurrects.
|
||||||
|
// Intentionally never reset: every caller of clearPersistedTreeCaches()
|
||||||
|
// immediately navigates away with a full page load
|
||||||
|
// (window.location.replace/href), so this module instance is torn down anyway.
|
||||||
|
// Only PERSISTENCE stops — the in-memory atoms keep working, so the UI stays
|
||||||
|
// intact during the brief pre-redirect window.
|
||||||
|
let persistenceDisabled = false;
|
||||||
|
|
||||||
|
function writeNow(key: string, value: SpaceTreeNode[]): void {
|
||||||
|
try {
|
||||||
|
const serialized = JSON.stringify(value);
|
||||||
|
if (serialized.length > MAX_SERIALIZED_LENGTH) {
|
||||||
|
// Warn ONCE, like the quota branch below: a >4M-char tree re-serializes on
|
||||||
|
// every ~500ms debounce tick while it's edited, so an un-gated warn would
|
||||||
|
// spam the console on each flush.
|
||||||
|
if (!writeFailureWarned) {
|
||||||
|
writeFailureWarned = true;
|
||||||
|
console.warn("[tree] cached tree too large to persist; skipping", key);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
localStorage.setItem(key, serialized);
|
||||||
|
} catch (err) {
|
||||||
|
// QuotaExceededError, private mode, jsdom shims without working storage…
|
||||||
|
// The cache is best-effort: warn once, keep the in-memory tree working.
|
||||||
|
if (!writeFailureWarned) {
|
||||||
|
writeFailureWarned = true;
|
||||||
|
console.warn("[tree] failed to persist tree cache", err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Exported so tests can force the debounced write synchronously; production
|
||||||
|
// code must never need it (the beforeunload hook below covers reloads).
|
||||||
|
export function flushPendingTreeDataWrites(): void {
|
||||||
|
if (flushTimer !== null) {
|
||||||
|
clearTimeout(flushTimer);
|
||||||
|
flushTimer = null;
|
||||||
|
}
|
||||||
|
if (persistenceDisabled) {
|
||||||
|
// Belt-and-braces: after logout nothing may reach localStorage, even via
|
||||||
|
// the beforeunload flush racing the redirect. Drop anything queued.
|
||||||
|
pendingWrites.clear();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
for (const [key, value] of pendingWrites) {
|
||||||
|
writeNow(key, value);
|
||||||
|
}
|
||||||
|
pendingWrites.clear();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Logout hygiene: the tree cache stores PAGE TITLES, so leaving it behind
|
||||||
|
// would keep them readable in localStorage on a shared machine after logout.
|
||||||
|
// Sweep by key prefix (not just the current scope) so stale scopes — old
|
||||||
|
// users, the `anon:anon` fallback — are purged too. Pending debounced writes
|
||||||
|
// are DISCARDED first (not flushed): a queued write firing after the sweep
|
||||||
|
// would silently resurrect a removed key.
|
||||||
|
export function clearPersistedTreeCaches(): void {
|
||||||
|
// Disable persistence FIRST so no write can be queued (or flushed) between
|
||||||
|
// the sweep below and the full-page navigation every caller performs next.
|
||||||
|
persistenceDisabled = true;
|
||||||
|
if (flushTimer !== null) {
|
||||||
|
clearTimeout(flushTimer);
|
||||||
|
flushTimer = null;
|
||||||
|
}
|
||||||
|
pendingWrites.clear();
|
||||||
|
try {
|
||||||
|
// Collect matching keys BEFORE removing: deleting while iterating
|
||||||
|
// `localStorage.key(i)` shifts the indices and skips entries.
|
||||||
|
const keysToRemove: string[] = [];
|
||||||
|
for (let i = 0; i < localStorage.length; i++) {
|
||||||
|
const key = localStorage.key(i);
|
||||||
|
if (
|
||||||
|
key !== null &&
|
||||||
|
(key.startsWith(TREE_DATA_KEY_PREFIX) ||
|
||||||
|
key.startsWith(OPEN_TREE_NODES_KEY_PREFIX))
|
||||||
|
) {
|
||||||
|
keysToRemove.push(key);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (const key of keysToRemove) {
|
||||||
|
localStorage.removeItem(key);
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Best-effort: disabled storage / jsdom shims must never break logout.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Flush the pending debounced write on unload so a reload right after a tree
|
||||||
|
// change doesn't lose the newest state (the debounce would otherwise eat it).
|
||||||
|
if (
|
||||||
|
typeof window !== "undefined" &&
|
||||||
|
typeof window.addEventListener === "function"
|
||||||
|
) {
|
||||||
|
window.addEventListener("beforeunload", flushPendingTreeDataWrites);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Custom sync storage for the tree cache. Deliberately NO `subscribe` key:
|
||||||
|
// cross-tab sync would REPLACE this tab's tree wholesale and clobber in-flight
|
||||||
|
// lazy loads; websockets already keep every open tab live. Each tab keeps its
|
||||||
|
// own in-memory tree — localStorage only seeds the next boot.
|
||||||
|
const treeDataStorage = {
|
||||||
|
getItem: (key: string, initialValue: SpaceTreeNode[]): SpaceTreeNode[] => {
|
||||||
|
// Defensive: jsdom test shims may lack methods, stored JSON may be
|
||||||
|
// corrupted or of a wrong shape. Any failure falls back to the empty tree.
|
||||||
|
try {
|
||||||
|
const raw = localStorage.getItem(key);
|
||||||
|
if (raw === null) return initialValue;
|
||||||
|
const parsed = JSON.parse(raw);
|
||||||
|
return Array.isArray(parsed) ? (parsed as SpaceTreeNode[]) : initialValue;
|
||||||
|
} catch {
|
||||||
|
return initialValue;
|
||||||
|
}
|
||||||
|
},
|
||||||
|
setItem: (key: string, newValue: SpaceTreeNode[]): void => {
|
||||||
|
// After logout the cache must stay purged: neither queue the write nor arm
|
||||||
|
// a new flush timer (see persistenceDisabled above). The in-memory atom
|
||||||
|
// value is unaffected — only the localStorage mirror is frozen.
|
||||||
|
if (persistenceDisabled) return;
|
||||||
|
pendingWrites.set(key, newValue);
|
||||||
|
if (flushTimer !== null) clearTimeout(flushTimer);
|
||||||
|
flushTimer = setTimeout(flushPendingTreeDataWrites, WRITE_DEBOUNCE_MS);
|
||||||
|
},
|
||||||
|
removeItem: (key: string): void => {
|
||||||
|
pendingWrites.delete(key);
|
||||||
|
try {
|
||||||
|
localStorage.removeItem(key);
|
||||||
|
} catch {
|
||||||
|
/* best-effort cache — ignore */
|
||||||
|
}
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
// One persisted tree per (workspace, user) — same scoping rationale as the
|
||||||
|
// open-map atom (accounts sharing a browser origin must not leak trees).
|
||||||
|
// `getOnInit: true` reads localStorage synchronously at atom init, so the very
|
||||||
|
// first render already has the cached tree — no blank-then-jump sidebar.
|
||||||
|
const treeDataFamily = atomFamily((scopeKey: string) =>
|
||||||
|
atomWithStorage<SpaceTreeNode[]>(
|
||||||
|
`${TREE_DATA_KEY_PREFIX}${scopeKey}`,
|
||||||
|
[],
|
||||||
|
treeDataStorage,
|
||||||
|
{ getOnInit: true },
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Public facade — same read value (SpaceTreeNode[]) and same setter shape
|
||||||
|
// (value OR functional updater) as the previous in-memory atom, transparently
|
||||||
|
// routed to the persisted tree of the current workspace/user.
|
||||||
|
export const treeDataAtom = atom(
|
||||||
|
(get) => get(treeDataFamily(get(scopeKeyAtom))),
|
||||||
|
(
|
||||||
|
get,
|
||||||
|
set,
|
||||||
|
update: SpaceTreeNode[] | ((prev: SpaceTreeNode[]) => SpaceTreeNode[]),
|
||||||
|
) => {
|
||||||
|
const target = treeDataFamily(get(scopeKeyAtom));
|
||||||
|
const next =
|
||||||
|
typeof update === "function"
|
||||||
|
? (update as (prev: SpaceTreeNode[]) => SpaceTreeNode[])(get(target))
|
||||||
|
: update;
|
||||||
|
set(target, next);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
// Atom
|
// Atom
|
||||||
export const appendNodeChildrenAtom = atom(
|
export const appendNodeChildrenAtom = atom(
|
||||||
|
|||||||
@@ -0,0 +1,222 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { createRef } from "react";
|
||||||
|
import { render, act, waitFor, cleanup } from "@testing-library/react";
|
||||||
|
|
||||||
|
// --- Mocks for the heavy / networked module graph ---------------------------
|
||||||
|
// Same isolation strategy as space-tree.expand-all.test.tsx: everything that
|
||||||
|
// would otherwise need a real server / router / DnD stack is mocked. Here we
|
||||||
|
// additionally CAPTURE the DocTree props (onToggle + data) so the test can
|
||||||
|
// drive a lazy-load expand exactly as a row click would, and we control
|
||||||
|
// fetchAllAncestorChildren to assert the fresh fetch happens.
|
||||||
|
|
||||||
|
const fetchAllAncestorChildrenMock = vi.fn();
|
||||||
|
|
||||||
|
// Holder mutated by the DocTree stub each render so the test can read the
|
||||||
|
// latest tree it was handed and invoke its onToggle callback.
|
||||||
|
const docTree: {
|
||||||
|
onToggle?: (id: string, isOpen: boolean) => void | Promise<void>;
|
||||||
|
data: unknown[];
|
||||||
|
} = { data: [] };
|
||||||
|
|
||||||
|
vi.mock("@/features/page/services/page-service.ts", () => ({
|
||||||
|
getSpaceTree: vi.fn(),
|
||||||
|
getPageBreadcrumbs: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||||
|
// No root pages and no further pages — the server data-load effect stays
|
||||||
|
// inert (isDataLoaded never flips), so refreshOpenBranches never runs and the
|
||||||
|
// test exercises ONLY the boot-prune + handleToggle lazy-load path against
|
||||||
|
// the hydrated cache we seed into the atom below.
|
||||||
|
useGetRootSidebarPagesQuery: () => ({
|
||||||
|
data: undefined,
|
||||||
|
hasNextPage: false,
|
||||||
|
fetchNextPage: vi.fn(),
|
||||||
|
isFetching: false,
|
||||||
|
}),
|
||||||
|
usePageQuery: () => ({ data: undefined }),
|
||||||
|
fetchAllAncestorChildren: (...args: unknown[]) =>
|
||||||
|
fetchAllAncestorChildrenMock(...args),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/features/page/tree/hooks/use-tree-mutation.ts", () => ({
|
||||||
|
useTreeMutation: () => ({ handleMove: vi.fn() }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@mantine/notifications", () => ({
|
||||||
|
notifications: { show: vi.fn() },
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("react-i18next", () => ({
|
||||||
|
useTranslation: () => ({ t: (key: string) => key }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("react-router-dom", () => ({
|
||||||
|
useParams: () => ({ pageSlug: undefined }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/lib", () => ({
|
||||||
|
extractPageSlugId: () => undefined,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/lib/config.ts", () => ({
|
||||||
|
isCompactPageTreeEnabled: () => false,
|
||||||
|
}));
|
||||||
|
|
||||||
|
// Capture the props DocTree is rendered with instead of rendering anything.
|
||||||
|
vi.mock("./doc-tree", () => ({
|
||||||
|
DocTree: (props: { onToggle: (id: string, isOpen: boolean) => void; data: unknown[] }) => {
|
||||||
|
docTree.onToggle = props.onToggle;
|
||||||
|
docTree.data = props.data;
|
||||||
|
return null;
|
||||||
|
},
|
||||||
|
ROW_HEIGHT_COMPACT: 28,
|
||||||
|
ROW_HEIGHT_STANDARD: 32,
|
||||||
|
}));
|
||||||
|
vi.mock("./space-tree-row", () => ({
|
||||||
|
SpaceTreeRow: () => null,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@mantine/core", () => ({
|
||||||
|
Text: ({ children }: { children?: unknown }) => children ?? null,
|
||||||
|
}));
|
||||||
|
|
||||||
|
// In-memory open-map (the real one is localStorage-backed and crashes under the
|
||||||
|
// jsdom shim). Empty at start of each test -> every branch is COLLAPSED, which
|
||||||
|
// is exactly the state we need to prove the boot-prune. `scopeKeyAtom` is
|
||||||
|
// re-exported because the persisted tree-data atom resolves its scope through it.
|
||||||
|
vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
|
||||||
|
const { atom } = await import("jotai");
|
||||||
|
type OpenMap = Record<string, boolean>;
|
||||||
|
const base = atom<OpenMap>({});
|
||||||
|
const openTreeNodesAtom = atom(
|
||||||
|
(get) => get(base),
|
||||||
|
(get, set, update: OpenMap | ((prev: OpenMap) => OpenMap)) => {
|
||||||
|
const next =
|
||||||
|
typeof update === "function"
|
||||||
|
? (update as (prev: OpenMap) => OpenMap)(get(base))
|
||||||
|
: update;
|
||||||
|
set(base, next);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
const scopeKeyAtom = atom(() => "test-workspace:test-user");
|
||||||
|
return { openTreeNodesAtom, scopeKeyAtom };
|
||||||
|
});
|
||||||
|
|
||||||
|
import SpaceTree, { SpaceTreeApi } from "./space-tree";
|
||||||
|
import {
|
||||||
|
treeDataAtom,
|
||||||
|
flushPendingTreeDataWrites,
|
||||||
|
} from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||||
|
import { createStore, Provider } from "jotai";
|
||||||
|
import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||||
|
|
||||||
|
// The scopeKeyAtom mock resolves to this fixed scope, so the persisted
|
||||||
|
// tree-data atom hydrates from exactly this localStorage key at mount
|
||||||
|
// (getOnInit + atomWithStorage's onMount both read it).
|
||||||
|
const CACHE_KEY = "treeData:v1:test-workspace:test-user";
|
||||||
|
|
||||||
|
function child(
|
||||||
|
id: string,
|
||||||
|
parentPageId: string,
|
||||||
|
hasChildren = false,
|
||||||
|
): SpaceTreeNode {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
slugId: `slug-${id}`,
|
||||||
|
name: id,
|
||||||
|
position: "a0",
|
||||||
|
spaceId: "space-1",
|
||||||
|
parentPageId,
|
||||||
|
hasChildren,
|
||||||
|
children: [],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// A hydrated boot cache: a COLLAPSED branch (not in the open-map) that still
|
||||||
|
// carries a stale cached child — the exact shape a previous session left behind
|
||||||
|
// after the branch was expanded then collapsed then persisted.
|
||||||
|
function cachedTreeWithCollapsedBranch(): SpaceTreeNode[] {
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
id: "branch",
|
||||||
|
slugId: "slug-branch",
|
||||||
|
name: "branch",
|
||||||
|
position: "a0",
|
||||||
|
spaceId: "space-1",
|
||||||
|
parentPageId: null as unknown as string,
|
||||||
|
hasChildren: true,
|
||||||
|
children: [child("stale", "branch")],
|
||||||
|
},
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
fetchAllAncestorChildrenMock.mockReset();
|
||||||
|
docTree.onToggle = undefined;
|
||||||
|
docTree.data = [];
|
||||||
|
// Flush any pending debounced write from a previous test before clearing.
|
||||||
|
flushPendingTreeDataWrites();
|
||||||
|
try {
|
||||||
|
localStorage.clear?.();
|
||||||
|
} catch {
|
||||||
|
/* fresh store per test isolates state */
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("SpaceTree boot-cache prune (#159 #8 stale collapsed children)", () => {
|
||||||
|
it("drops a collapsed cached branch's children on boot and fetches fresh on first expand", async () => {
|
||||||
|
// Server returns FRESH children on the lazy-load: the stale cached child is
|
||||||
|
// gone, a renamed/new one takes its place.
|
||||||
|
fetchAllAncestorChildrenMock.mockResolvedValue([child("fresh", "branch")]);
|
||||||
|
|
||||||
|
// Simulate the localStorage-hydrated boot cache: seed the persisted key
|
||||||
|
// BEFORE mount so the atom hydrates it (store.set would be clobbered by
|
||||||
|
// atomWithStorage's onMount re-reading storage — this is the real path).
|
||||||
|
localStorage.setItem(
|
||||||
|
CACHE_KEY,
|
||||||
|
JSON.stringify(cachedTreeWithCollapsedBranch()),
|
||||||
|
);
|
||||||
|
|
||||||
|
const store = createStore();
|
||||||
|
const ref = createRef<SpaceTreeApi>();
|
||||||
|
render(
|
||||||
|
<Provider store={store}>
|
||||||
|
<SpaceTree ref={ref} spaceId="space-1" readOnly={false} />
|
||||||
|
</Provider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Boot-prune ran at mount: the COLLAPSED branch's cached children were
|
||||||
|
// dropped to the unloaded shape ([]), so the stale child is no longer there.
|
||||||
|
const branchAfterBoot = docTree.data.find(
|
||||||
|
(n) => (n as SpaceTreeNode).id === "branch",
|
||||||
|
) as SpaceTreeNode;
|
||||||
|
expect(branchAfterBoot.children).toEqual([]);
|
||||||
|
expect(branchAfterBoot.hasChildren).toBe(true);
|
||||||
|
|
||||||
|
// First expand of the collapsed branch after boot must lazy-load fresh
|
||||||
|
// children (before this fix the cached children were kept and the fetch
|
||||||
|
// was skipped, showing stale data).
|
||||||
|
await act(async () => {
|
||||||
|
await docTree.onToggle!("branch", true);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(fetchAllAncestorChildrenMock).toHaveBeenCalledTimes(1);
|
||||||
|
expect(fetchAllAncestorChildrenMock).toHaveBeenCalledWith({
|
||||||
|
pageId: "branch",
|
||||||
|
spaceId: "space-1",
|
||||||
|
});
|
||||||
|
|
||||||
|
// The fresh children replaced the stale cache in the live tree.
|
||||||
|
await waitFor(() => {
|
||||||
|
const branch = store
|
||||||
|
.get(treeDataAtom)
|
||||||
|
.find((n) => n.id === "branch")!;
|
||||||
|
expect(branch.children.map((c) => c.id)).toEqual(["fresh"]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -71,7 +71,8 @@ vi.mock("@mantine/core", () => ({
|
|||||||
// getOnInit), which crashes under jsdom's localStorage shim here. Swap in a
|
// getOnInit), which crashes under jsdom's localStorage shim here. Swap in a
|
||||||
// plain in-memory atom with the same read value (OpenMap) and the same setter
|
// plain in-memory atom with the same read value (OpenMap) and the same setter
|
||||||
// shape (value OR functional updater) so the component's open-state logic runs
|
// shape (value OR functional updater) so the component's open-state logic runs
|
||||||
// unchanged while staying inside the test store.
|
// unchanged while staying inside the test store. `scopeKeyAtom` is also
|
||||||
|
// re-exported (the real module exports it for the persisted tree-data atom).
|
||||||
vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
|
vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
|
||||||
const { atom } = await import("jotai");
|
const { atom } = await import("jotai");
|
||||||
type OpenMap = Record<string, boolean>;
|
type OpenMap = Record<string, boolean>;
|
||||||
@@ -86,11 +87,17 @@ vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
|
|||||||
set(base, next);
|
set(base, next);
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
return { openTreeNodesAtom };
|
// Fixed scope key: the tree-data atom family resolves through this, so all
|
||||||
|
// tests read/write the same (empty at start of each test) storage key.
|
||||||
|
const scopeKeyAtom = atom(() => "test-workspace:test-user");
|
||||||
|
return { openTreeNodesAtom, scopeKeyAtom };
|
||||||
});
|
});
|
||||||
|
|
||||||
import SpaceTree, { SpaceTreeApi } from "./space-tree";
|
import SpaceTree, { SpaceTreeApi } from "./space-tree";
|
||||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
import {
|
||||||
|
treeDataAtom,
|
||||||
|
flushPendingTreeDataWrites,
|
||||||
|
} from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||||
import { openTreeNodesAtom } from "@/features/page/tree/atoms/open-tree-nodes-atom.ts";
|
import { openTreeNodesAtom } from "@/features/page/tree/atoms/open-tree-nodes-atom.ts";
|
||||||
import { createStore, Provider } from "jotai";
|
import { createStore, Provider } from "jotai";
|
||||||
import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||||
@@ -134,6 +141,10 @@ function renderTree(store: ReturnType<typeof createStore>) {
|
|||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
getSpaceTreeMock.mockReset();
|
getSpaceTreeMock.mockReset();
|
||||||
notificationsShowMock.mockReset();
|
notificationsShowMock.mockReset();
|
||||||
|
// The tree-data atom persists via a ~500 ms trailing debounce; flush it NOW
|
||||||
|
// (cancelling the timer) so a previous test's pending write can't land in
|
||||||
|
// storage mid-test after the clear below.
|
||||||
|
flushPendingTreeDataWrites();
|
||||||
// jsdom's localStorage shim here lacks `clear`; guard it. Each test uses a
|
// jsdom's localStorage shim here lacks `clear`; guard it. Each test uses a
|
||||||
// fresh jotai store anyway, so cross-test open-state never leaks.
|
// fresh jotai store anyway, so cross-test open-state never leaks.
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ import {
|
|||||||
openBranches,
|
openBranches,
|
||||||
closeIds,
|
closeIds,
|
||||||
loadedOpenBranchIds,
|
loadedOpenBranchIds,
|
||||||
|
pruneCollapsedChildren,
|
||||||
} from "@/features/page/tree/utils/utils.ts";
|
} from "@/features/page/tree/utils/utils.ts";
|
||||||
import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||||
import { treeModel } from "@/features/page/tree/model/tree-model";
|
import { treeModel } from "@/features/page/tree/model/tree-model";
|
||||||
@@ -199,45 +200,81 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
|
|||||||
const openIdsRef = useRef(openIds);
|
const openIdsRef = useRef(openIds);
|
||||||
openIdsRef.current = openIds;
|
openIdsRef.current = openIds;
|
||||||
|
|
||||||
// Reconnect refresh (#159 #8): on a socket reconnect, re-fetch and reconcile
|
// Boot-cache hygiene (#159 #8): the localStorage-hydrated tree carries the
|
||||||
// the children of every currently-open, already-loaded branch of THIS space,
|
// children of every branch ever expanded, including ones now COLLAPSED. Their
|
||||||
|
// first expand would skip the lazy-load and render stale children (a
|
||||||
|
// rename/move/delete missed while offline). Drop the cached children of every
|
||||||
|
// COLLAPSED branch ONCE at mount so its first expand fetches fresh via
|
||||||
|
// handleToggle — exactly as it did before the tree was cached. OPEN branches
|
||||||
|
// keep their children and are refreshed by refreshOpenBranches instead, so
|
||||||
|
// this runs before any expand and never double-fetches an open branch.
|
||||||
|
const prunedBootCacheRef = useRef(false);
|
||||||
|
useEffect(() => {
|
||||||
|
if (prunedBootCacheRef.current) return;
|
||||||
|
prunedBootCacheRef.current = true;
|
||||||
|
setData((prev) => pruneCollapsedChildren(prev, openIdsRef.current));
|
||||||
|
}, [setData]);
|
||||||
|
|
||||||
|
// Re-fetch and reconcile the children of every currently-open, already-loaded
|
||||||
|
// branch of THIS space. Shared by the socket reconnect handler and the
|
||||||
|
// post-load cache refresh below. The ROOT level is reconciled separately by
|
||||||
|
// the root-query refetch + mergeRootTrees; an UNLOADED branch is skipped
|
||||||
|
// (lazy-load fetches it fresh on expand). Reads refs so it always sees the
|
||||||
|
// latest tree/open-state/space without re-creating the callback.
|
||||||
|
const refreshOpenBranches = useCallback(async () => {
|
||||||
|
const effectSpaceId = spaceIdRef.current;
|
||||||
|
const branchIds = loadedOpenBranchIds(
|
||||||
|
dataRef.current.filter((n) => n?.spaceId === effectSpaceId),
|
||||||
|
openIdsRef.current,
|
||||||
|
);
|
||||||
|
if (branchIds.length === 0) return;
|
||||||
|
for (const id of branchIds) {
|
||||||
|
try {
|
||||||
|
// `fresh: true` bypasses the 30-min sidebar-pages cache so the
|
||||||
|
// reconcile sees the server's CURRENT children (handler-order
|
||||||
|
// independent — no reliance on the global reconnect invalidation).
|
||||||
|
const fresh = await fetchAllAncestorChildren(
|
||||||
|
{ pageId: id, spaceId: effectSpaceId },
|
||||||
|
{ fresh: true },
|
||||||
|
);
|
||||||
|
if (spaceIdRef.current !== effectSpaceId) return; // space switched
|
||||||
|
setData((prev) => treeModel.reconcileChildren(prev, id, fresh));
|
||||||
|
} catch (err) {
|
||||||
|
console.error("[tree] open branch refresh failed", err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}, [setData]);
|
||||||
|
|
||||||
|
// Reconnect refresh (#159 #8): on a socket reconnect, refresh open branches
|
||||||
// so a move/rename/delete that happened INSIDE a loaded branch while events
|
// so a move/rename/delete that happened INSIDE a loaded branch while events
|
||||||
// were missed (laptop sleep / wifi gap) is reflected instead of left stale.
|
// were missed (laptop sleep / wifi gap) is reflected instead of left stale.
|
||||||
// The ROOT level is reconciled separately by the root-query refetch +
|
// No first-connect guard is needed: space-tree usually mounts AFTER the
|
||||||
// mergeRootTrees; an UNLOADED branch is skipped (lazy-load fetches it fresh on
|
// initial connect, so every `connect` it sees is a reconnect; the rare
|
||||||
// expand). No first-connect guard is needed: space-tree usually mounts AFTER
|
|
||||||
// the initial connect, so every `connect` it sees is a reconnect; the rare
|
|
||||||
// initial-connect case has an empty tree, so the refresh is a harmless no-op.
|
// initial-connect case has an empty tree, so the refresh is a harmless no-op.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!socket) return;
|
if (!socket) return;
|
||||||
const onConnect = async () => {
|
const onConnect = () => {
|
||||||
const effectSpaceId = spaceIdRef.current;
|
refreshOpenBranches();
|
||||||
const branchIds = loadedOpenBranchIds(
|
|
||||||
dataRef.current.filter((n) => n?.spaceId === effectSpaceId),
|
|
||||||
openIdsRef.current,
|
|
||||||
);
|
|
||||||
if (branchIds.length === 0) return;
|
|
||||||
for (const id of branchIds) {
|
|
||||||
try {
|
|
||||||
// `fresh: true` bypasses the 30-min sidebar-pages cache so the
|
|
||||||
// reconcile sees the server's CURRENT children (handler-order
|
|
||||||
// independent — no reliance on the global reconnect invalidation).
|
|
||||||
const fresh = await fetchAllAncestorChildren(
|
|
||||||
{ pageId: id, spaceId: effectSpaceId },
|
|
||||||
{ fresh: true },
|
|
||||||
);
|
|
||||||
if (spaceIdRef.current !== effectSpaceId) return; // space switched
|
|
||||||
setData((prev) => treeModel.reconcileChildren(prev, id, fresh));
|
|
||||||
} catch (err) {
|
|
||||||
console.error("[tree] reconnect branch refresh failed", err);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
};
|
};
|
||||||
socket.on("connect", onConnect);
|
socket.on("connect", onConnect);
|
||||||
return () => {
|
return () => {
|
||||||
socket.off("connect", onConnect);
|
socket.off("connect", onConnect);
|
||||||
};
|
};
|
||||||
}, [socket, setData]);
|
}, [socket, refreshOpenBranches]);
|
||||||
|
|
||||||
|
// Post-load cache refresh: the sidebar paints instantly from the
|
||||||
|
// localStorage-cached tree, so children of open branches may be stale. Once
|
||||||
|
// the server root set has been merged for this space (isDataLoaded flips
|
||||||
|
// true), refresh every open, already-loaded branch ONCE per space per mount.
|
||||||
|
// dataRef.current is already up to date here: refs are assigned during
|
||||||
|
// render, and this effect runs after the merge-triggered re-render commit.
|
||||||
|
const refreshedSpacesRef = useRef<Set<string>>(new Set());
|
||||||
|
useEffect(() => {
|
||||||
|
if (!isDataLoaded) return;
|
||||||
|
if (refreshedSpacesRef.current.has(spaceId)) return;
|
||||||
|
refreshedSpacesRef.current.add(spaceId);
|
||||||
|
refreshOpenBranches();
|
||||||
|
}, [isDataLoaded, spaceId, refreshOpenBranches]);
|
||||||
|
|
||||||
const handleToggle = useCallback(
|
const handleToggle = useCallback(
|
||||||
async (id: string, isOpen: boolean) => {
|
async (id: string, isOpen: boolean) => {
|
||||||
@@ -333,12 +370,17 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={classes.treeContainer}>
|
<div className={classes.treeContainer}>
|
||||||
|
{/* "No pages yet" only after the SERVER confirmed the space is empty —
|
||||||
|
never while just the localStorage cache is empty. */}
|
||||||
{isDataLoaded && filteredData.length === 0 && (
|
{isDataLoaded && filteredData.length === 0 && (
|
||||||
<Text size="xs" c="dimmed" py="xs" px="sm">
|
<Text size="xs" c="dimmed" py="xs" px="sm">
|
||||||
{t("No pages yet")}
|
{t("No pages yet")}
|
||||||
</Text>
|
</Text>
|
||||||
)}
|
)}
|
||||||
{isDataLoaded && filteredData.length > 0 && (
|
{/* Cache-first paint: render as soon as ANY data exists (synchronous
|
||||||
|
localStorage hydration) instead of waiting for the server round-trip;
|
||||||
|
the background merge/refresh reconciles it afterwards. */}
|
||||||
|
{filteredData.length > 0 && (
|
||||||
<DocTree<SpaceTreeNode>
|
<DocTree<SpaceTreeNode>
|
||||||
data={filteredData}
|
data={filteredData}
|
||||||
openIds={openIds}
|
openIds={openIds}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import {
|
|||||||
closeIds,
|
closeIds,
|
||||||
mergeRootTrees,
|
mergeRootTrees,
|
||||||
loadedOpenBranchIds,
|
loadedOpenBranchIds,
|
||||||
|
pruneCollapsedChildren,
|
||||||
sortPositionKeys,
|
sortPositionKeys,
|
||||||
pageToTreeNode,
|
pageToTreeNode,
|
||||||
} from "./utils";
|
} from "./utils";
|
||||||
@@ -438,3 +439,62 @@ describe("loadedOpenBranchIds (#159 #8 reconnect refresh targets)", () => {
|
|||||||
expect(ids.sort()).toEqual(["a", "a1"]);
|
expect(ids.sort()).toEqual(["a", "a1"]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("pruneCollapsedChildren", () => {
|
||||||
|
// Signature: pruneCollapsedChildren(tree: SpaceTreeNode[], openIds:
|
||||||
|
// ReadonlySet<string>): SpaceTreeNode[]. Collapsed nodes (id NOT in openIds)
|
||||||
|
// are reset to `children: []` (hasChildren untouched); open nodes keep their
|
||||||
|
// children but are recursed into so a collapsed branch nested under an open
|
||||||
|
// one is still pruned.
|
||||||
|
//
|
||||||
|
// Fixture:
|
||||||
|
// open "p" (in openIds, hasChildren)
|
||||||
|
// └─ collapsed "c" (NOT in openIds) with STALE child "g"
|
||||||
|
// collapsed "t" (NOT in openIds) with child "t1"
|
||||||
|
// Only "p" is open.
|
||||||
|
function fixture() {
|
||||||
|
const grandchild = treeNode("g"); // stale, cached under the collapsed child
|
||||||
|
const collapsedChild = treeNode("c", [grandchild]);
|
||||||
|
const openParent = treeNode("p", [collapsedChild]);
|
||||||
|
const topCollapsed = treeNode("t", [treeNode("t1")]);
|
||||||
|
return { openParent, collapsedChild, topCollapsed };
|
||||||
|
}
|
||||||
|
|
||||||
|
it("keeps an OPEN parent's children and recurses to prune a nested collapsed branch; prunes a top-level collapsed node", () => {
|
||||||
|
const { openParent, topCollapsed } = fixture();
|
||||||
|
const tree = [openParent, topCollapsed];
|
||||||
|
const result = pruneCollapsedChildren(tree, new Set(["p"]));
|
||||||
|
|
||||||
|
// (a) OPEN parent keeps its children (not cleared) and hasChildren stays true.
|
||||||
|
const p = result[0];
|
||||||
|
expect(p.id).toBe("p");
|
||||||
|
expect(p.hasChildren).toBe(true);
|
||||||
|
expect(p.children).toHaveLength(1);
|
||||||
|
|
||||||
|
// (b) The nested COLLAPSED child under the open parent is pruned to
|
||||||
|
// `children: []` by the recursion, with hasChildren preserved. This is the
|
||||||
|
// open-keep + recurse branch that F1's empty-open-set fixture never hits.
|
||||||
|
const c = p.children[0];
|
||||||
|
expect(c.id).toBe("c");
|
||||||
|
expect(c.children).toEqual([]);
|
||||||
|
expect(c.hasChildren).toBe(true);
|
||||||
|
|
||||||
|
// (c) The top-level collapsed node is pruned to `children: []`, hasChildren kept.
|
||||||
|
const t = result[1];
|
||||||
|
expect(t.id).toBe("t");
|
||||||
|
expect(t.children).toEqual([]);
|
||||||
|
expect(t.hasChildren).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not mutate the input tree (returns fresh nodes)", () => {
|
||||||
|
const { openParent, collapsedChild, topCollapsed } = fixture();
|
||||||
|
const tree = [openParent, topCollapsed];
|
||||||
|
pruneCollapsedChildren(tree, new Set(["p"]));
|
||||||
|
|
||||||
|
// Originals are untouched: the collapsed child still carries its stale grandchild.
|
||||||
|
expect(collapsedChild.children).toHaveLength(1);
|
||||||
|
expect(collapsedChild.children[0].id).toBe("g");
|
||||||
|
expect(openParent.children[0]).toBe(collapsedChild);
|
||||||
|
expect(topCollapsed.children).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -293,6 +293,41 @@ export function loadedOpenBranchIds(
|
|||||||
return ids;
|
return ids;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Boot-cache hygiene (#159 #8): the persisted tree keeps the children of EVERY
|
||||||
|
* branch ever expanded — collapsing a branch never prunes them. So on reload a
|
||||||
|
* COLLAPSED branch hydrates with its old cached children, and `handleToggle`
|
||||||
|
* skips the lazy-load on first expand (children already present) → it shows
|
||||||
|
* STALE children (renamed / moved / deleted while the user was offline) with no
|
||||||
|
* reconcile. `refreshOpenBranches` only refreshes OPEN branches, so collapsed
|
||||||
|
* ones slip through.
|
||||||
|
*
|
||||||
|
* Fix: drop the cached children of every node NOT in the persisted open-set,
|
||||||
|
* resetting it to the canonical UNLOADED shape (`children: []`, `hasChildren`
|
||||||
|
* untouched — see pageToTreeNode). Its first expand then lazy-loads fresh, just
|
||||||
|
* as it did before the tree was cached to localStorage. OPEN branches keep
|
||||||
|
* their children (refreshOpenBranches reconciles those, so they must not be
|
||||||
|
* dropped here) and are recursed into so a collapsed branch nested under an
|
||||||
|
* open one is pruned too.
|
||||||
|
*/
|
||||||
|
export function pruneCollapsedChildren(
|
||||||
|
tree: SpaceTreeNode[],
|
||||||
|
openIds: ReadonlySet<string>,
|
||||||
|
): SpaceTreeNode[] {
|
||||||
|
return tree.map((node) => {
|
||||||
|
const hasLoadedChildren = !!node.children && node.children.length > 0;
|
||||||
|
if (!openIds.has(node.id)) {
|
||||||
|
// Collapsed: drop the whole cached subtree so it reads as unloaded.
|
||||||
|
return hasLoadedChildren ? { ...node, children: [] } : node;
|
||||||
|
}
|
||||||
|
// Open: keep it, but recurse into its children (a nested collapsed branch
|
||||||
|
// must still be pruned).
|
||||||
|
return hasLoadedChildren
|
||||||
|
? { ...node, children: pruneCollapsedChildren(node.children, openIds) }
|
||||||
|
: node;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// Collect every node id in the tree (roots, branches, leaves). Used by
|
// Collect every node id in the tree (roots, branches, leaves). Used by
|
||||||
// collapseAll to clear the open-state map for all current-space nodes.
|
// collapseAll to clear the open-state map for all current-space nodes.
|
||||||
export function collectAllIds(nodes: SpaceTreeNode[]): string[] {
|
export function collectAllIds(nodes: SpaceTreeNode[]): string[] {
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import axios, { AxiosInstance } from "axios";
|
import axios, { AxiosInstance } from "axios";
|
||||||
import APP_ROUTE from "@/lib/app-route.ts";
|
import APP_ROUTE from "@/lib/app-route.ts";
|
||||||
import { isCloud } from "@/lib/config.ts";
|
import { isCloud } from "@/lib/config.ts";
|
||||||
|
import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom";
|
||||||
|
|
||||||
const api: AxiosInstance = axios.create({
|
const api: AxiosInstance = axios.create({
|
||||||
baseURL: "/api",
|
baseURL: "/api",
|
||||||
@@ -71,6 +72,12 @@ function redirectToLogin() {
|
|||||||
"/invites",
|
"/invites",
|
||||||
];
|
];
|
||||||
if (!exemptPaths.some((path) => window.location.pathname.startsWith(path))) {
|
if (!exemptPaths.some((path) => window.location.pathname.startsWith(path))) {
|
||||||
|
// Forced logout (401 / expired session) must purge the persisted sidebar
|
||||||
|
// tree caches too: they contain page titles, and on a shared machine most
|
||||||
|
// sessions end via cookie expiry — not the logout button — so this is the
|
||||||
|
// only cleanup that runs on that path. It also disables further cache
|
||||||
|
// persistence until the full page load below.
|
||||||
|
clearPersistedTreeCaches();
|
||||||
const redirectTo = window.location.pathname;
|
const redirectTo = window.location.pathname;
|
||||||
if (redirectTo === APP_ROUTE.HOME) {
|
if (redirectTo === APP_ROUTE.HOME) {
|
||||||
window.location.href = APP_ROUTE.AUTH.LOGIN;
|
window.location.href = APP_ROUTE.AUTH.LOGIN;
|
||||||
|
|||||||
@@ -303,6 +303,11 @@ describe('buildSystemPrompt page-changed note (#274)', () => {
|
|||||||
expect(prompt).toContain(NOTE_MARKER);
|
expect(prompt).toContain(NOTE_MARKER);
|
||||||
expect(prompt).toContain('-old line');
|
expect(prompt).toContain('-old line');
|
||||||
expect(prompt).toContain('+new line');
|
expect(prompt).toContain('+new line');
|
||||||
|
// Strengthened note (#274): instructs a fresh re-read via getPage and steers
|
||||||
|
// the agent toward small, targeted edits instead of a full-page overwrite.
|
||||||
|
expect(prompt).toContain('getPage');
|
||||||
|
expect(prompt.toLowerCase()).toContain('targeted');
|
||||||
|
expect(prompt).toContain('editPageText');
|
||||||
// Inside the safety sandwich: the trailing SAFETY block follows the note.
|
// Inside the safety sandwich: the trailing SAFETY block follows the note.
|
||||||
expect(prompt.lastIndexOf(SAFETY_MARKER)).toBeGreaterThan(
|
expect(prompt.lastIndexOf(SAFETY_MARKER)).toBeGreaterThan(
|
||||||
prompt.indexOf(NOTE_MARKER),
|
prompt.indexOf(NOTE_MARKER),
|
||||||
|
|||||||
@@ -85,11 +85,17 @@ const INTERRUPT_NOTE =
|
|||||||
const PAGE_CHANGED_NOTE =
|
const PAGE_CHANGED_NOTE =
|
||||||
'NOTE: The user edited the open page AFTER your last response in this ' +
|
'NOTE: The user edited the open page AFTER your last response in this ' +
|
||||||
'conversation, so any copy of that page you produced or remember from earlier ' +
|
'conversation, so any copy of that page you produced or remember from earlier ' +
|
||||||
'is now STALE. The unified diff below shows exactly what changed since you last ' +
|
'is now STALE and must not be reused. Before you edit the page, you MUST first ' +
|
||||||
'spoke (lines starting with "-" were removed, "+" were added) and is the source ' +
|
're-read its current content with the getPage tool and base your work on that ' +
|
||||||
'of truth. Preserve the user\'s edits: build on the current page, do not revert ' +
|
'live version — never on your earlier copy or on the transcript. The unified ' +
|
||||||
'or overwrite their changes. If you need the full up-to-date page, re-read it ' +
|
'diff below shows exactly what the user changed since you last spoke (lines ' +
|
||||||
'with the getPage tool before editing.';
|
'starting with "-" were removed, "+" were added) and is the source of truth. ' +
|
||||||
|
'Preserve every one of the user\'s edits: make the smallest change that ' +
|
||||||
|
'satisfies the request using the targeted edit tools (editPageText, patchNode, ' +
|
||||||
|
'insertNode, deleteNode) rather than replacing the whole page, and do not ' +
|
||||||
|
'revert, drop, or overwrite anything the user changed. If a full rewrite is ' +
|
||||||
|
'truly unavoidable, start from the current getPage content and carry over all ' +
|
||||||
|
'of the user\'s edits.';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sanitize a value interpolated into a prompt XML-ish attribute (e.g.
|
* Sanitize a value interpolated into a prompt XML-ish attribute (e.g.
|
||||||
|
|||||||
@@ -356,6 +356,32 @@ describe('flushAssistant', () => {
|
|||||||
expect(flushed.toolCalls).not.toBeNull();
|
expect(flushed.toolCalls).not.toBeNull();
|
||||||
expect(flushed.metadata.error).toBe('boom');
|
expect(flushed.metadata.error).toBe('boom');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #274 observability: the page-change diff the agent saw this turn is persisted
|
||||||
|
// to metadata.pageChanged when a non-empty diff was injected, and omitted when
|
||||||
|
// the diff is empty/whitespace or the arg is not supplied.
|
||||||
|
it('persists metadata.pageChanged when a non-empty diff was injected', () => {
|
||||||
|
const f = flushAssistant([], '', 'completed', {
|
||||||
|
pageChanged: { title: 'Doc', diff: '@@ -1 +1 @@\n-old\n+new' },
|
||||||
|
});
|
||||||
|
expect(f.metadata.pageChanged).toEqual({
|
||||||
|
title: 'Doc',
|
||||||
|
diff: '@@ -1 +1 @@\n-old\n+new',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('omits metadata.pageChanged for an empty/whitespace diff or a missing arg', () => {
|
||||||
|
const whitespace = flushAssistant([], '', 'completed', {
|
||||||
|
pageChanged: { title: 'Doc', diff: ' \n ' },
|
||||||
|
});
|
||||||
|
expect('pageChanged' in whitespace.metadata).toBe(false);
|
||||||
|
|
||||||
|
const nullArg = flushAssistant([], '', 'completed', { pageChanged: null });
|
||||||
|
expect('pageChanged' in nullArg.metadata).toBe(false);
|
||||||
|
|
||||||
|
const omitted = flushAssistant([], '', 'streaming');
|
||||||
|
expect('pageChanged' in omitted.metadata).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -685,7 +685,7 @@ export class AiChatService implements OnModuleInit {
|
|||||||
// no-op (guarded below) so the turn still streams to the user.
|
// no-op (guarded below) so the turn still streams to the user.
|
||||||
let assistantId: string | undefined;
|
let assistantId: string | undefined;
|
||||||
try {
|
try {
|
||||||
const seed = flushAssistant([], '', 'streaming');
|
const seed = flushAssistant([], '', 'streaming', { pageChanged });
|
||||||
const seeded = await this.aiChatMessageRepo.insert({
|
const seeded = await this.aiChatMessageRepo.insert({
|
||||||
chatId,
|
chatId,
|
||||||
workspaceId: workspace.id,
|
workspaceId: workspace.id,
|
||||||
@@ -720,7 +720,7 @@ export class AiChatService implements OnModuleInit {
|
|||||||
await this.aiChatMessageRepo.update(
|
await this.aiChatMessageRepo.update(
|
||||||
assistantId,
|
assistantId,
|
||||||
workspace.id,
|
workspace.id,
|
||||||
flushAssistant(capturedSteps, '', 'streaming'),
|
flushAssistant(capturedSteps, '', 'streaming', { pageChanged }),
|
||||||
{ onlyIfStreaming: true },
|
{ onlyIfStreaming: true },
|
||||||
);
|
);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
@@ -860,6 +860,7 @@ export class AiChatService implements OnModuleInit {
|
|||||||
// resolved from the admin-configured provider settings (in
|
// resolved from the admin-configured provider settings (in
|
||||||
// closure scope here). Omitted/0 = no limit.
|
// closure scope here). Omitted/0 = no limit.
|
||||||
maxContextTokens: resolved?.chatContextWindow,
|
maxContextTokens: resolved?.chatContextWindow,
|
||||||
|
pageChanged,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
// Lifecycle: release the external MCP clients leased for this turn.
|
// Lifecycle: release the external MCP clients leased for this turn.
|
||||||
@@ -911,6 +912,7 @@ export class AiChatService implements OnModuleInit {
|
|||||||
await finalizeAssistant(
|
await finalizeAssistant(
|
||||||
flushAssistant(capturedSteps, inProgressText, 'error', {
|
flushAssistant(capturedSteps, inProgressText, 'error', {
|
||||||
error: errorText,
|
error: errorText,
|
||||||
|
pageChanged,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
await closeExternalClients();
|
await closeExternalClients();
|
||||||
@@ -940,7 +942,9 @@ export class AiChatService implements OnModuleInit {
|
|||||||
`steps=${steps.length}`,
|
`steps=${steps.length}`,
|
||||||
);
|
);
|
||||||
await finalizeAssistant(
|
await finalizeAssistant(
|
||||||
flushAssistant(capturedSteps, inProgressText, 'aborted'),
|
flushAssistant(capturedSteps, inProgressText, 'aborted', {
|
||||||
|
pageChanged,
|
||||||
|
}),
|
||||||
);
|
);
|
||||||
await closeExternalClients();
|
await closeExternalClients();
|
||||||
// Advance the page snapshot even on abort (#274): an agent edit that
|
// Advance the page snapshot even on abort (#274): an agent edit that
|
||||||
@@ -1506,6 +1510,7 @@ export function flushAssistant(
|
|||||||
contextTokens?: number;
|
contextTokens?: number;
|
||||||
maxContextTokens?: number;
|
maxContextTokens?: number;
|
||||||
error?: string;
|
error?: string;
|
||||||
|
pageChanged?: { title: string; diff: string } | null;
|
||||||
},
|
},
|
||||||
): AssistantFlush {
|
): AssistantFlush {
|
||||||
const finished = capturedSteps ?? [];
|
const finished = capturedSteps ?? [];
|
||||||
@@ -1538,6 +1543,15 @@ export function flushAssistant(
|
|||||||
if (extra?.maxContextTokens)
|
if (extra?.maxContextTokens)
|
||||||
metadata.maxContextTokens = extra.maxContextTokens;
|
metadata.maxContextTokens = extra.maxContextTokens;
|
||||||
if (extra?.error) metadata.error = extra.error;
|
if (extra?.error) metadata.error = extra.error;
|
||||||
|
// Persist the page-change diff the agent saw this turn (#274 observability),
|
||||||
|
// so history / the Markdown export can show what the user changed. Only when
|
||||||
|
// a non-empty diff was actually injected into the prompt this turn.
|
||||||
|
if (extra?.pageChanged && extra.pageChanged.diff?.trim().length) {
|
||||||
|
metadata.pageChanged = {
|
||||||
|
title: extra.pageChanged.title,
|
||||||
|
diff: extra.pageChanged.diff,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
content: stepsText + trailing,
|
content: stepsText + trailing,
|
||||||
|
|||||||
@@ -269,6 +269,168 @@ describe('buildChatMarkdown (server) — structure', () => {
|
|||||||
expect(md).toContain('**⚠️ Error:** 401: Unauthorized');
|
expect(md).toContain('**⚠️ Error:** 401: Unauthorized');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #274 observability: an assistant row whose turn started with a user edit to
|
||||||
|
// the open page carries metadata.pageChanged = { title, diff }; the export
|
||||||
|
// renders the diff the agent saw, before the message body.
|
||||||
|
it('renders the persisted page-change diff block for an assistant row', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [
|
||||||
|
row({
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'answer',
|
||||||
|
metadata: {
|
||||||
|
pageChanged: { title: 'Doc', diff: '@@ -1 +1 @@\n-old\n+new' },
|
||||||
|
} as never,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(md).toContain(
|
||||||
|
'The user edited this page before this turn; the diff the agent saw:',
|
||||||
|
);
|
||||||
|
expect(md).toContain('("Doc")');
|
||||||
|
expect(md).toContain('-old');
|
||||||
|
expect(md).toContain('+new');
|
||||||
|
// The diff sits before the message body (chronological: change, then reply).
|
||||||
|
expect(md.indexOf('-old')).toBeLessThan(md.indexOf('answer'));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not render the page-change block when metadata.pageChanged is absent', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [row({ role: 'assistant', content: 'answer' })],
|
||||||
|
});
|
||||||
|
expect(md).not.toContain(
|
||||||
|
'The user edited this page before this turn; the diff the agent saw:',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// #288 F1/F2: an empty page title must render the BARE heading with no
|
||||||
|
// `("…")` suffix (the `pc.title ? … : …` false branch).
|
||||||
|
it('renders the page-change heading with no title suffix when title is empty', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [
|
||||||
|
row({
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'answer',
|
||||||
|
metadata: {
|
||||||
|
pageChanged: { title: '', diff: '@@ -1 +1 @@\n-old\n+new' },
|
||||||
|
} as never,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
// Bare heading, single line, no parenthesized title.
|
||||||
|
expect(md).toContain(
|
||||||
|
'> **📝 The user edited this page before this turn; the diff the agent saw:**',
|
||||||
|
);
|
||||||
|
expect(md).not.toContain('("');
|
||||||
|
expect(md).toContain('-old');
|
||||||
|
});
|
||||||
|
|
||||||
|
// #288 F1: the page title is UNTRUSTED cross-user data, so a title carrying a
|
||||||
|
// newline / backtick / `"` / `<`/`>` must be neutralized by escapeAttr before
|
||||||
|
// it is interpolated into the `> **…**` blockquote heading — otherwise it
|
||||||
|
// could break the blockquote onto multiple lines or inject markup/HTML into
|
||||||
|
// the downloaded .md. escapeAttr strips `<>"` and collapses whitespace runs to
|
||||||
|
// a single space, so `Ev"il\n> `x` <b>` becomes ``Evil `x` b``.
|
||||||
|
it('escapes an untrusted page title in the page-change heading', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [
|
||||||
|
row({
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'answer',
|
||||||
|
metadata: {
|
||||||
|
pageChanged: {
|
||||||
|
title: 'Ev"il\n> `x` <b>',
|
||||||
|
diff: '@@ -1 +1 @@\n-old\n+new',
|
||||||
|
},
|
||||||
|
} as never,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
// The heading stays a single blockquote line with the escaped title.
|
||||||
|
expect(md).toContain(
|
||||||
|
'> **📝 The user edited this page before this turn; the diff the agent saw: ("Evil `x` b")**',
|
||||||
|
);
|
||||||
|
// No raw attribute/markup breakers survived from the title.
|
||||||
|
expect(md).not.toContain('Ev"il');
|
||||||
|
expect(md).not.toContain('<b>');
|
||||||
|
});
|
||||||
|
|
||||||
|
// #288 review F1: escapeAttr ALONE is insufficient for this MARKDOWN sink —
|
||||||
|
// link/image syntax survives it. A cross-user title with `` /
|
||||||
|
// `[phish](url)` must NOT become a working remote image or clickable link in
|
||||||
|
// the downloaded .md; markdownHeadingSafe backslash-escapes `[`/`]` so both are
|
||||||
|
// inert. (Non-vacuous: fails against the escapeAttr-only version, which left
|
||||||
|
// `](https://` intact.)
|
||||||
|
it('neutralizes markdown link/image syntax in an untrusted page title', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [
|
||||||
|
row({
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'answer',
|
||||||
|
metadata: {
|
||||||
|
pageChanged: {
|
||||||
|
title:
|
||||||
|
' and [click](https://phish.example)',
|
||||||
|
diff: '@@ -1 +1 @@\n-old\n+new',
|
||||||
|
},
|
||||||
|
} as never,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
// No WORKING image/link syntax survives — the `[…]` sits escaped as `\[…\]`,
|
||||||
|
// so the unescaped ``: after escaping the
|
||||||
|
// literal `\](https://` still contains `](https://` as a raw substring — that
|
||||||
|
// check would false-fail even though the link is inert.)
|
||||||
|
expect(md).not.toContain(';
|
||||||
|
expect(md).not.toContain('[click](');
|
||||||
|
// The brackets are backslash-escaped, so `[text](url)`/`` are inert.
|
||||||
|
expect(md).toContain('\\[');
|
||||||
|
expect(md).toContain('\\]');
|
||||||
|
// The heading stays a SINGLE blockquote line (no newline injected).
|
||||||
|
const headingLine = md
|
||||||
|
.split('\n')
|
||||||
|
.find((l) => l.includes('the diff the agent saw:'));
|
||||||
|
expect(headingLine).toBeDefined();
|
||||||
|
expect(headingLine).toContain('\\[x\\]');
|
||||||
|
expect(headingLine).toContain('\\[click\\]');
|
||||||
|
});
|
||||||
|
|
||||||
|
// #288 internal review Finding 2: a NON-empty title made up entirely of
|
||||||
|
// escapeAttr breakers (`<>"`) escapes to '' — the ternary must then fall to the
|
||||||
|
// BARE heading with NO `("…")` suffix. Locks the ternary-on-escaped-value
|
||||||
|
// behavior (distinct from the empty-string input test above).
|
||||||
|
it('renders the bare heading for a title that escapes to empty', () => {
|
||||||
|
const md = buildChatMarkdown({
|
||||||
|
title: 'T',
|
||||||
|
chatId: 'c',
|
||||||
|
rows: [
|
||||||
|
row({
|
||||||
|
role: 'assistant',
|
||||||
|
content: 'answer',
|
||||||
|
metadata: {
|
||||||
|
pageChanged: { title: '<>"', diff: '@@ -1 +1 @@\n-old\n+new' },
|
||||||
|
} as never,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(md).toContain(
|
||||||
|
'> **📝 The user edited this page before this turn; the diff the agent saw:**',
|
||||||
|
);
|
||||||
|
expect(md).not.toContain('("');
|
||||||
|
expect(md).toContain('-old');
|
||||||
|
});
|
||||||
|
|
||||||
it('escapes embedded triple-backtick fences with a longer delimiter', () => {
|
it('escapes embedded triple-backtick fences with a longer delimiter', () => {
|
||||||
const md = buildChatMarkdown({
|
const md = buildChatMarkdown({
|
||||||
title: 'T',
|
title: 'T',
|
||||||
|
|||||||
@@ -15,6 +15,7 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { AiChatMessage } from '@docmost/db/types/entity.types';
|
import type { AiChatMessage } from '@docmost/db/types/entity.types';
|
||||||
|
import { escapeAttr } from './ai-chat.prompt';
|
||||||
|
|
||||||
/** Supported export label languages. Defaults to English. */
|
/** Supported export label languages. Defaults to English. */
|
||||||
export type ExportLang = 'en' | 'ru';
|
export type ExportLang = 'en' | 'ru';
|
||||||
@@ -63,6 +64,7 @@ const LABELS: Record<
|
|||||||
tools: Record<string, string>;
|
tools: Record<string, string>;
|
||||||
ranTool: (name: string) => string;
|
ranTool: (name: string) => string;
|
||||||
stillGenerating: string;
|
stillGenerating: string;
|
||||||
|
pageEditedByUser: string;
|
||||||
}
|
}
|
||||||
> = {
|
> = {
|
||||||
en: {
|
en: {
|
||||||
@@ -83,6 +85,8 @@ const LABELS: Record<
|
|||||||
ranTool: (name) => `Ran tool ${name}`,
|
ranTool: (name) => `Ran tool ${name}`,
|
||||||
stillGenerating:
|
stillGenerating:
|
||||||
'This message is still being generated — the export captured a partial, in-progress response.',
|
'This message is still being generated — the export captured a partial, in-progress response.',
|
||||||
|
pageEditedByUser:
|
||||||
|
'The user edited this page before this turn; the diff the agent saw:',
|
||||||
},
|
},
|
||||||
ru: {
|
ru: {
|
||||||
untitled: 'Без названия',
|
untitled: 'Без названия',
|
||||||
@@ -102,9 +106,29 @@ const LABELS: Record<
|
|||||||
ranTool: (name) => `Выполнил инструмент ${name}`,
|
ranTool: (name) => `Выполнил инструмент ${name}`,
|
||||||
stillGenerating:
|
stillGenerating:
|
||||||
'Это сообщение всё ещё генерируется — экспорт захватил частичный, незавершённый ответ.',
|
'Это сообщение всё ещё генерируется — экспорт захватил частичный, незавершённый ответ.',
|
||||||
|
pageEditedByUser:
|
||||||
|
'Пользователь изменил страницу перед этим ходом; дифф, который видел агент:',
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Make an untrusted title safe to interpolate into a Markdown blockquote
|
||||||
|
* HEADING. escapeAttr() neutralizes the XML/HTML breakers (`<` `>` `"`) and
|
||||||
|
* collapses whitespace for the PROMPT sink (`page="…"`), but this export sink is
|
||||||
|
* MARKDOWN — link/image syntax survives escapeAttr. So additionally backslash-
|
||||||
|
* escape `[` and `]`: that disables both `[text](url)` links and ``
|
||||||
|
* images, so a cross-user title like `` or `[phish](http://evil)`
|
||||||
|
* cannot inject a remote (auto-loading) image or a clickable link into the
|
||||||
|
* downloaded .md disguised as a trusted system annotation. A bare `(url)` with no
|
||||||
|
* preceding `[]` is inert Markdown, so brackets are the only security-critical
|
||||||
|
* characters here. (We leave backticks to escapeAttr's whitespace pass — a title
|
||||||
|
* shown as inline code cannot escape the blockquote line or load a resource, so
|
||||||
|
* it is not a security concern for this sink.)
|
||||||
|
*/
|
||||||
|
function markdownHeadingSafe(title: string): string {
|
||||||
|
return escapeAttr(title).replace(/[[\]]/g, (m) => `\\${m}`);
|
||||||
|
}
|
||||||
|
|
||||||
/** True for AI SDK tool parts (static `tool-*` or `dynamic-tool`). */
|
/** True for AI SDK tool parts (static `tool-*` or `dynamic-tool`). */
|
||||||
function isToolPart(type: string): boolean {
|
function isToolPart(type: string): boolean {
|
||||||
return type.startsWith('tool-') || type === 'dynamic-tool';
|
return type.startsWith('tool-') || type === 'dynamic-tool';
|
||||||
@@ -208,6 +232,23 @@ function rowParts(row: AiChatMessage): ExportPart[] {
|
|||||||
: [{ type: 'text', text: row.content ?? '' }];
|
: [{ type: 'text', text: row.content ?? '' }];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The persisted page-change diff the agent saw this turn (#274), when any. */
|
||||||
|
function pageChangedOf(
|
||||||
|
row: AiChatMessage,
|
||||||
|
): { title: string; diff: string } | undefined {
|
||||||
|
const meta = (row.metadata ?? {}) as {
|
||||||
|
pageChanged?: { title?: string; diff?: string };
|
||||||
|
};
|
||||||
|
const pc = meta.pageChanged;
|
||||||
|
if (pc && typeof pc.diff === 'string' && pc.diff.trim().length > 0) {
|
||||||
|
return {
|
||||||
|
title: typeof pc.title === 'string' ? pc.title : '',
|
||||||
|
diff: pc.diff,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Serialize a chat to a Markdown string from its persisted rows. Source = DB
|
* Serialize a chat to a Markdown string from its persisted rows. Source = DB
|
||||||
* ONLY (no live client state). A row whose `status` is still 'streaming' is an
|
* ONLY (no live client state). A row whose `status` is still 'streaming' is an
|
||||||
@@ -266,6 +307,26 @@ export function buildChatMarkdown(args: {
|
|||||||
blocks.push(`<!-- ${iso} -->`);
|
blocks.push(`<!-- ${iso} -->`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Page-change observability (#274): show the diff the agent saw at the start
|
||||||
|
// of this turn, before its response, so the export reflects the stale-page
|
||||||
|
// warning the model received.
|
||||||
|
const pc = pageChangedOf(row);
|
||||||
|
if (pc) {
|
||||||
|
// The page title is UNTRUSTED cross-user data (a collaborative page's title
|
||||||
|
// controllable by another user). escapeAttr() alone (the prompt sink) is
|
||||||
|
// INSUFFICIENT here: this is a MARKDOWN sink, so we neutralize link/image
|
||||||
|
// syntax too (backslash-escaping `[`/`]`) before interpolating it into this
|
||||||
|
// `> **…**` blockquote heading — otherwise `` / `[phish](url)` would
|
||||||
|
// inject a remote image or clickable link into the downloaded .md. An
|
||||||
|
// all-`<>"` title escapes to empty and correctly falls to the bare heading.
|
||||||
|
// The diff body is already safe via fence(). (#288 review F1.)
|
||||||
|
const safeTitle = markdownHeadingSafe(pc.title);
|
||||||
|
const heading = safeTitle
|
||||||
|
? `${L.pageEditedByUser} ("${safeTitle}")`
|
||||||
|
: L.pageEditedByUser;
|
||||||
|
blocks.push(`> **📝 ${heading}**\n\n${fence(pc.diff, 'diff')}`);
|
||||||
|
}
|
||||||
|
|
||||||
blocks.push(...renderMessageParts(rowParts(row), lang));
|
blocks.push(...renderMessageParts(rowParts(row), lang));
|
||||||
|
|
||||||
// A still-'streaming' row is an interrupted/in-progress turn captured by the
|
// A still-'streaming' row is an interrupted/in-progress turn captured by the
|
||||||
|
|||||||
@@ -449,7 +449,9 @@ export function applyAlignment(container: HTMLElement, align: string) {
|
|||||||
// the next line when the viewport is narrow. The right/bottom padding
|
// the next line when the viewport is narrow. The right/bottom padding
|
||||||
// provides the gap between images in a row and between wrapped rows;
|
// provides the gap between images in a row and between wrapped rows;
|
||||||
// vertical-align: top keeps rows of different-height images aligned by
|
// vertical-align: top keeps rows of different-height images aligned by
|
||||||
// their top edge.
|
// their top edge. Horizontal centering of the whole row is handled by the
|
||||||
|
// client stylesheet (media.css) via a :has() rule on the parent block
|
||||||
|
// container, since the row has no wrapper element of its own.
|
||||||
container.style.display = "inline-block";
|
container.style.display = "inline-block";
|
||||||
container.style.verticalAlign = "top";
|
container.style.verticalAlign = "top";
|
||||||
container.style.padding = "0 10px 10px 0";
|
container.style.padding = "0 10px 10px 0";
|
||||||
|
|||||||
Reference in New Issue
Block a user