Compare commits

..

16 Commits

Author SHA1 Message Date
vvzvlad 916b24e3ff Merge pull request 'fix(ui): collapse global sidebar to a drawer below 992px (tablet layout overflow, #291)' (#292) from fix/responsive-tablet-sidebar into develop
Reviewed-on: #292
2026-07-02 22:52:11 +03:00
vvzvlad ecf022ffca Merge pull request 'fix(editor): не подставлять типографику повторно после её отмены (Ctrl+Z)' (#296) from fix/typography-undo-resubstitution into develop
Reviewed-on: #296
2026-07-02 22:51:50 +03:00
vvzvlad 62af116271 Merge pull request 'fix(editor): copy tables to clipboard as Markdown, not newline-joined cells' (#297) from fix/table-clipboard-markdown into develop
Reviewed-on: #297
2026-07-02 22:51:26 +03:00
agent_coder db9ed51e01 fix(#296 review): symmetric diff-bound normalization + tests (F1/F2)
F2: findChangedRange only normalized the repeated-content INSERTION case
(oldTo<start), leaving the symmetric DELETION case (newTo<start) to return a
degenerate DocChange (newTo<from). Push BOTH ends forward by start-min(oldTo,newTo)
so the range stays non-degenerate (from<=oldTo, from<=newTo), matching ProseMirror's
diff bounds. The insertion case is byte-identical to before (min=oldTo → oldTo→start,
newTo→newTo+delta); the deletion/both-below cases are fixed. Never spuriously arms
the guard (arming needs oldTo>from AND newTo>from; normalization leaves exactly one
end ==from).
F1: add custom-typography.test.ts (15 tests) via the real Editor path (mirrors
intentional-clear.test.ts): findChangedRange normalization (insertion + the fixed
deletion), mapRangeThroughChange release/boundary/shift, and arming (local
undo-replace arms; remote y-sync change-origin does NOT; ordinary edit does NOT).
Adds test-only exports (undoGuardKey, findChangedRange, mapRangeThroughChange).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 22:18:14 +03:00
agent_coder c452902432 test(#297 review): pin table->markdown at the output level (F1)
The existing tests assert only the classifier flags ({asMarkdown, wrapBareRows}),
not the resulting markdown. Add two output-level tests via htmlToMarkdown mirroring
the serializer's real path: (a) a header-less bare-rows selection wrapped as
<table><tbody><tr>… yields a VALID GFM pipe table (GFM plugin synthesizes an empty
header + separator), and (b) a whole table with a header round-trips to a proper
pipe table with header/separator/data rows. Both are non-vacuous — they fail
against the old one-value-per-line serialization (no separator row, no pipes).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 22:10:30 +03:00
agent_coder 731a4f0dca refactor(#292 review): extract NAVBAR_COLLAPSE_BREAKPOINT constant (F2)
The AppShell navbar breakpoint and both burger toggles' hiddenFrom/visibleFrom
must be equal, or the sidebar becomes unreachable on tablet widths (the round-1
regression). A comment guarded that before; now a shared const does. Add
NAVBAR_COLLAPSE_BREAKPOINT='md' to sidebar-atom.ts and reference it from the navbar
breakpoint (global-app-shell) + both toggles (app-header). aside.breakpoint and the
sm brand/search gates are intentionally separate contracts, left untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 22:02:37 +03:00
claude_code 895173b176 Merge branch 'develop' of https://gitea.vvzvlad.xyz/vvzvlad/gitmost into develop 2026-07-02 19:31:05 +03:00
vvzvlad 45d5ae1601 Merge pull request '[feature][ai-chat] Наблюдаемость page_changed-диффа в истории/экспорте + усиление ноты против перезаписи правок' (#288) from feature/ai-chat-page-change-observability into develop
Reviewed-on: #288
2026-07-02 19:30:53 +03:00
claude_code ec30e6c08a docs(agents): update Gitea MCP workflow details in agents guide
Add clarification that pushing commits is git‑native while PR creation uses the Gitea MCP, replace curl/tea examples with MCP method calls, update API table entries, and revise issue creation instructions accordingly.
2026-07-02 19:20:56 +03:00
claude_code db9f29c16b fix(editor): copy tables to clipboard as Markdown, not newline-joined cells
clipboardTextSerializer only produced Markdown for lists, so copying a table
and pasting into a plain-text/Markdown target emitted one cell value per line
(ProseMirror's default text serializer). Route tables through htmlToMarkdown
(turndown + GFM) as well.

- Extract the decision into a pure, exported classifyClipboardSelection()
  helper; the existing list rule (2+ items) is preserved exactly.
- Handle whole-table selections (top-level `table` node) and partial cell
  selections (bare `tableRow` nodes), wrapping bare rows in <table><tbody> so
  the GFM turndown rule detects them.
- Add unit tests for classifyClipboardSelection (6 cases).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 18:56:57 +03:00
claude-stand fa439d7c7b fix(ui): match sidebar toggle breakpoint to navbar (md) so the tablet drawer opens
Follow-up to the navbar sm->md change on this branch: the two header sidebar
toggles were still gated at sm, so in the 768-991 band the DESKTOP toggle was
shown while the navbar used the MOBILE drawer collapse state — clicking it
flipped the wrong atom and the drawer could not be opened (sidebar unreachable
at 768/820, caught by QA). Gate the mobile toggle hiddenFrom=md and the desktop
toggle visibleFrom=md so the mobile toggle drives the drawer across the whole
tablet band.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 18:19:59 +03:00
claude_code 82411f8707 fix(editor): не подставлять типографику повторно после её отмены (Ctrl+Z)
После срабатывания авто-подстановки Typography (например «1/2 » → «½») и её
отмены через Ctrl+Z повторное нажатие пробела снова триггерило то же input-rule
и подставляло символ заново.

Добавлено клиентское расширение CustomTypography (обёртка над
@tiptap/extension-typography) с ProseMirror-плагином «undo guard»:
- запоминает диапазон текста, восстановленный отменой (undo/redo), и подавляет
  typography input-rules, чьё совпадение пересекается с этим диапазоном, пока
  восстановленный текст не отредактируют;
- поддерживает обе системы истории: prosemirror-history (шаблонные редакторы) и
  yjs UndoManager (основной collab-редактор). Undo в yjs приходит как замена
  всего документа, поэтому регион вычисляется диффом документов
  (findDiffStart/findDiffEnd), а не по step-map;
- детекция yjs-транзакций — через импортированный ySyncPluginKey и канонический
  isChangeOrigin, без хрупких строковых ключей.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-02 17:14:19 +03:00
vvzvlad af481d401a Merge pull request 'feat(editor): center inline image rows by default via CSS :has()' (#295) from image-inline-center into develop 2026-07-02 16:50:32 +03:00
claude-stand affa32cbaa fix(ui): collapse global sidebar to a drawer below 992px (tablet layout overflow)
At tablet widths (~768px) the fixed ~300px global sidebar stayed pinned, leaving
too little room for content: the settings tables (Members etc.) overflowed the
offset content area and pushed the Role/actions columns off-screen with no
horizontal scroll (unreachable). Raise the AppShell navbar (and page aside)
breakpoint from `sm` (768px) to `md` (992px) so the whole tablet band uses the
toggle drawer (closed by default) and content gets the full width.

Verified with Playwright screenshots: 768px settings/members now fits all columns
(table right 736<768, no overflow); desktop (>=992px) unchanged (sidebar pinned,
content offset); mobile unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 16:28:03 +03:00
agent_coder 438ef091f9 fix(#288 review): markdown-safe-escape the untrusted page title in chat export
F1: pc.title (untrusted cross-user page title) was interpolated raw into the
markdown export heading. Reusing escapeAttr alone (the prompt sink's XML-attribute
sanitizer, strips < > ") is insufficient here because the sink is MARKDOWN: link
/image syntax survives, so a title like ![x](http://evil) or [phish](http://evil)
injects a remote image / clickable link into the downloaded .md disguised as a
trusted system annotation. Add markdownHeadingSafe() = escapeAttr() + backslash-
escape [ and ] (disables both [text](url) and ![text](url); a bare (url) is inert).
F2: cover the title branch — a title that collapses to empty via escapeAttr falls
to the bare heading (no ("")), and a link/image-injection title is neutralized
(non-vacuous vs the escapeAttr-only version).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 15:46:44 +03:00
claude_code c39fab70c1 feat(ai-chat): persist page-change diff to history and harden stale-page note
The #274 page_changed marker lived only in the ephemeral system prompt, so the
diff the agent saw was invisible in the chat export/history, and the note was
too weak — the agent still overwrote the user's manual edits with a full-page
replace.

- Persist the diff the agent saw as metadata.pageChanged on the assistant row
  (flushAssistant), threaded into all five flush call sites in stream(). Model
  replay (rowToUiMessage/rowParts) reads only metadata.parts, so the sibling
  never re-injects the note into the model context on later turns.
- Render the persisted diff as a labelled block (en/ru) before the message body
  in the server-side Markdown export (chat-markdown.util.ts).
- Strengthen PAGE_CHANGED_NOTE: mandate a fresh getPage re-read and targeted
  edits (editPageText/patchNode/insertNode/deleteNode) instead of a whole-page
  replace, and never revert or overwrite the user's edits.

Tests: prompt, export and service specs updated; 114 pass, tsc clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-02 14:31:41 +03:00
15 changed files with 1012 additions and 47 deletions
+32 -21
View File
@@ -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.
--- ---
@@ -12,6 +12,7 @@ import TopMenu from "@/components/layouts/global/top-menu.tsx";
import { Link } from "react-router-dom"; import { Link } from "react-router-dom";
import { useAtom } from "jotai"; import { useAtom } from "jotai";
import { import {
NAVBAR_COLLAPSE_BREAKPOINT,
desktopSidebarAtom, desktopSidebarAtom,
mobileSidebarAtom, mobileSidebarAtom,
} from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts"; } from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts";
@@ -53,7 +54,13 @@ export function AppHeader() {
aria-label={t("Sidebar toggle")} aria-label={t("Sidebar toggle")}
opened={mobileOpened} opened={mobileOpened}
onClick={toggleMobile} onClick={toggleMobile}
hiddenFrom="sm" // Must match the AppShell navbar breakpoint (md). The navbar
// collapses to the MOBILE drawer below md, so the mobile toggle
// (which flips mobileOpened) must be the one visible across the
// whole <md band — otherwise at 768-991 the desktop toggle showed
// but flipped the wrong atom, leaving the drawer unopenable (the
// regression from the initial sm->md navbar change).
hiddenFrom={NAVBAR_COLLAPSE_BREAKPOINT}
size="sm" size="sm"
/> />
</Tooltip> </Tooltip>
@@ -63,7 +70,7 @@ export function AppHeader() {
aria-label={t("Sidebar toggle")} aria-label={t("Sidebar toggle")}
opened={desktopOpened} opened={desktopOpened}
onClick={toggleDesktop} onClick={toggleDesktop}
visibleFrom="sm" visibleFrom={NAVBAR_COLLAPSE_BREAKPOINT}
size="sm" size="sm"
/> />
</Tooltip> </Tooltip>
@@ -6,6 +6,7 @@ import SettingsSidebar from "@/components/settings/settings-sidebar.tsx";
import { useAtom } from "jotai"; import { useAtom } from "jotai";
import { import {
APP_NAVBAR_ID, APP_NAVBAR_ID,
NAVBAR_COLLAPSE_BREAKPOINT,
asideStateAtom, asideStateAtom,
desktopSidebarAtom, desktopSidebarAtom,
mobileSidebarAtom, mobileSidebarAtom,
@@ -88,7 +89,13 @@ export default function GlobalAppShell({
header={{ height: 45 }} header={{ height: 45 }}
navbar={{ navbar={{
width: isSpaceRoute ? sidebarWidth : 300, width: isSpaceRoute ? sidebarWidth : 300,
breakpoint: "sm", // `md` (not `sm`): below 992px the fixed ~300px sidebar leaves too little
// room for content — the settings tables (Members/…) overflow the offset
// content area on tablet (~768px) and clip the Role/actions columns
// off-screen with no horizontal scroll. Collapsing the navbar to a toggle
// drawer across the whole tablet band frees the full width for content
// (the mobile drawer is closed by default, so nothing overlaps on load).
breakpoint: NAVBAR_COLLAPSE_BREAKPOINT,
collapsed: { collapsed: {
mobile: !mobileOpened, mobile: !mobileOpened,
desktop: !desktopOpened, desktop: !desktopOpened,
@@ -97,7 +104,7 @@ export default function GlobalAppShell({
aside={ aside={
isPageRoute && { isPageRoute && {
width: 420, width: 420,
breakpoint: "sm", breakpoint: "md",
collapsed: { mobile: !isAsideOpen, desktop: !isAsideOpen }, collapsed: { mobile: !isAsideOpen, desktop: !isAsideOpen },
} }
} }
@@ -7,6 +7,13 @@ import { atom } from "jotai";
// would create a shell -> chat-window -> shell import cycle). // would create a shell -> chat-window -> shell import cycle).
export const APP_NAVBAR_ID = "app-shell-navbar"; export const APP_NAVBAR_ID = "app-shell-navbar";
// Single source of truth for the navbar collapse breakpoint. The AppShell navbar
// `breakpoint` and BOTH burger toggles' `hiddenFrom`/`visibleFrom` MUST use this
// exact value: if they drift, the sidebar becomes unreachable on tablet widths
// (the round-1 regression of #292). Kept here so the shell and the header share
// one constant the compiler enforces, instead of three hand-synced string literals.
export const NAVBAR_COLLAPSE_BREAKPOINT = "md";
export const mobileSidebarAtom = atom<boolean>(false); export const mobileSidebarAtom = atom<boolean>(false);
export const desktopSidebarAtom = atomWithWebStorage<boolean>( export const desktopSidebarAtom = atomWithWebStorage<boolean>(
@@ -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);
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); 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
@@ -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),
+11 -5
View File
@@ -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 `![x](url)` /
// `[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:
'![x](https://attacker.example/t.png) 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 `![x](` image and `[click](` link markers are gone. (We
// deliberately do NOT assert `not.toContain('](https://')`: 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('![x](');
expect(md).not.toContain('[click](');
// The brackets are backslash-escaped, so `[text](url)`/`![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 `![text](url)`
* images, so a cross-user title like `![x](http://evil)` 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 `![x](url)` / `[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