Compare commits

..

1 Commits

Author SHA1 Message Date
agent_coder 9685074237 perf(collab): три фикса горячего пути сервера — connect-vs-unload гонка, двойная перекодировка, isDeepStrictEqual (замер)
Побочные находки инцидента #400 (правка большой таблицы через MCP подвешивает всё).

1. Гонка connect-vs-unload в @hocuspocus/server 3.4.4 (вероятный источник 25s
   connect-таймаутов): createDocument проверяет loadingDocuments/documents, но НЕ
   ждёт unloadingDocuments -> новое соединение может захендшейкаться на умирающий
   Document -> redis-sync идёт по пути 'doc не загружен', провайдер висит до
   таймаута. Апстрим (main) не починен. pnpm-патч (инфра как у yjs-патча): в начале
   createDocument await in-flight unload (обёрнут в try/catch — отклонённый unload
   не отравляет открытие, поведение как до патча), в ОБОИХ рантаймах (cjs+esm).
   Тест hocuspocus-unload-race: реальный createDocument с засеянным in-flight
   unload -> не грузит пока unload не осел; при откате патча тест краснеет.

2. Двойная перекодировка в onLoadDocument (persistence.extension.ts): хук строил
   НОВЫЙ Y.Doc и возвращал его -> hocuspocus делал applyUpdate(encodeStateAsUpdate)
   ВТОРОЙ раз (315КБ на каждую холодную загрузку); в JSON-ветке результат encode
   выбрасывался (мёртвый вызов). Теперь стейт применяется прямо в data.document,
   возврат undefined (hocuspocus мержит только при возврате Doc); мёртвый encode
   убран. Содержимое документа не меняется — только меньше encode/alloc.

3. isDeepStrictEqual по 84КБ JSON на каждом store: замерил — 1.32мс на 90КБ
   (immaterial, <50мс порога; доминируют fromYdoc+encodeStateAsUpdate). Изменений
   кода НЕТ по правилу задачи (dirty-флаг только при material).

closes #401

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 02:53:23 +03:00
13 changed files with 409 additions and 419 deletions
@@ -0,0 +1,140 @@
/**
* gitmost #401 — regression test for the connect-vs-unload race in
* @hocuspocus/server 3.4.4 (patched via patches/@hocuspocus__server@3.4.4.patch).
*
* The race (unpatched): when the last client disconnects, storeDocumentHooks'
* `finally` schedules an async `unloadDocument`. That unload runs its
* `beforeUnloadDocument` hooks asynchronously and, meanwhile, records an
* in-flight promise in `this.unloadingDocuments`. In the original 3.4.4
* `createDocument`, a NEW connection arriving in that window falls straight
* through to the `loadingDocuments`/`documents` checks — it never consults
* `unloadingDocuments`. So the new connection can start loading (or reuse) a
* document while the old instance is still being torn down; the re-check inside
* unload (`shouldUnloadDocument`, which sees 0 connections because async auth
* hooks have not registered the new connection yet) then deletes/destroys the
* doc out from under the freshly-connected client → orphaned Document → later
* redis-sync takes the "doc not loaded" path → sync never completes → the
* provider hangs until its ~25s timeout.
*
* The patch: `createDocument` first awaits any in-flight
* `unloadingDocuments.get(name)` before proceeding. Once that settles, the
* decision is deterministic — either the doc was fully unloaded (gone from
* `documents`, so a clean fresh load) or the unload aborted (healthy doc still
* in `documents`, reused). The new connection can never hand-shake onto an
* about-to-be-destroyed Document.
*
* These tests exercise the REAL patched `Hocuspocus.createDocument` (the class
* is directly constructible) by seeding `unloadingDocuments` with a controllable
* in-flight unload and observing that createDocument waits for it.
*/
import { Hocuspocus } from '@hocuspocus/server';
// A promise we can resolve on demand, to model an unload that is mid-flight.
function deferred<T = void>() {
let resolve!: (v: T) => void;
const promise = new Promise<T>((r) => {
resolve = r;
});
return { promise, resolve };
}
describe('gitmost #401 — hocuspocus createDocument awaits in-flight unload', () => {
it('does NOT start loading a new doc until the in-flight unload settles, then loads fresh', async () => {
const hp = new Hocuspocus();
const name = 'page.race';
// Observe loadDocument: on the unpatched code it is invoked synchronously
// within createDocument (before the unload settles); on the patched code it
// must be deferred until unloadingDocuments resolves.
const freshDoc = { name, __fresh: true } as any;
const loadSpy = jest
.spyOn(hp as any, 'loadDocument')
.mockResolvedValue(freshDoc);
// Model an unload in progress: an entry sits in unloadingDocuments and, when
// it completes, it removes the doc from `documents` (a real full unload).
const unload = deferred();
(hp as any).documents.set(name, { name, __dying: true });
(hp as any).unloadingDocuments.set(
name,
unload.promise.then(() => {
(hp as any).documents.delete(name);
}),
);
// Kick off a new connection's createDocument but do not await it yet.
const createPromise = (hp as any).createDocument(
name,
{},
'socket-1',
{ isAuthenticated: true, readOnly: false },
{},
);
// Let all currently-schedulable microtasks run. The patched createDocument is
// now parked on `await unloadingDocuments.get(name)`, so loadDocument must
// NOT have been called yet, and it must NOT have returned the dying doc.
await Promise.resolve();
await Promise.resolve();
expect(loadSpy).not.toHaveBeenCalled();
// The unload completes (doc removed from `documents`).
unload.resolve();
// createDocument now proceeds: sees no existing doc → fresh load.
const doc = await createPromise;
expect(loadSpy).toHaveBeenCalledTimes(1);
expect(doc).toBe(freshDoc);
// The freshly-loaded doc is the one registered — never the dying instance.
expect((hp as any).documents.get(name)).toBe(freshDoc);
});
it('reuses the live doc when the in-flight unload aborts (doc left in documents)', async () => {
const hp = new Hocuspocus();
const name = 'page.abort';
const loadSpy = jest.spyOn(hp as any, 'loadDocument');
// Model an unload that ABORTS (e.g. a new connection reappeared before the
// sync re-check): it settles WITHOUT deleting the doc from `documents`.
const unload = deferred();
const liveDoc = { name, __live: true } as any;
(hp as any).documents.set(name, liveDoc);
(hp as any).unloadingDocuments.set(name, unload.promise); // no-op unload
const createPromise = (hp as any).createDocument(
name,
{},
'socket-2',
{ isAuthenticated: true, readOnly: false },
{},
);
unload.resolve();
const doc = await createPromise;
// The still-present live doc is reused; no fresh load happened.
expect(doc).toBe(liveDoc);
expect(loadSpy).not.toHaveBeenCalled();
});
it('no in-flight unload → behaves normally (fresh load)', async () => {
const hp = new Hocuspocus();
const name = 'page.normal';
const freshDoc = { name } as any;
const loadSpy = jest
.spyOn(hp as any, 'loadDocument')
.mockResolvedValue(freshDoc);
const doc = await (hp as any).createDocument(
name,
{},
'socket-3',
{ isAuthenticated: true, readOnly: false },
{},
);
expect(loadSpy).toHaveBeenCalledTimes(1);
expect(doc).toBe(freshDoc);
});
});
@@ -0,0 +1,130 @@
/**
* gitmost #401 fix 2 — onLoadDocument applies the DB state directly into the
* hook's target document and returns undefined (instead of building a NEW Y.Doc
* and returning it, which made hocuspocus re-encode+apply the whole state a
* SECOND time on every cold load).
*
* These tests assert:
* - the hook mutates `data.document` in place so its content equals the DB doc,
* - onLoadDocument returns undefined (so hocuspocus keeps the mutated doc and
* does NOT run its own applyUpdate(encodeStateAsUpdate(...)) merge),
* - both the raw-ydoc branch and the json→ydoc conversion branch behave so.
*
* Returning undefined is the observable signal that the double-encode is gone
* (the old code returned a new Y.Doc, which made hocuspocus re-encode+apply the
* state a second time); we assert that contract rather than counting internal
* encode calls, which is brittle given the encodes inside toYdoc and the test's
* own `expected` fixtures.
*/
import * as Y from 'yjs';
import { Document } from '@hocuspocus/server';
import { TiptapTransformer } from '@hocuspocus/transformer';
import { PersistenceExtension } from './persistence.extension';
import { tiptapExtensions } from '../collaboration.util';
// A fresh hocuspocus Document (extends Y.Doc, adds isEmpty()) as hocuspocus
// hands to onLoadDocument on a cold load.
const freshDoc = () => new Document(`page.${PAGE_ID}`, {});
const PAGE_ID = '550e8400-e29b-41d4-a716-446655440000';
const doc = (text: string) => ({
type: 'doc',
content: [{ type: 'paragraph', content: [{ type: 'text', text }] }],
});
const jsonOf = (ydoc: Y.Doc) =>
TiptapTransformer.fromYdoc(ydoc, 'default');
describe('PersistenceExtension.onLoadDocument — #401 fix 2 (apply-into-hook-doc)', () => {
let ext: PersistenceExtension;
let pageRepo: { findById: jest.Mock };
beforeEach(() => {
pageRepo = { findById: jest.fn() };
ext = new PersistenceExtension(
pageRepo as any,
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
{} as any,
);
jest.spyOn(ext['logger'], 'debug').mockImplementation(() => undefined);
jest.spyOn(ext['logger'], 'warn').mockImplementation(() => undefined);
});
const load = (document: Document) =>
ext.onLoadDocument({ documentName: `page.${PAGE_ID}`, document } as any);
it('raw ydoc branch: mutates the hook doc to the DB state and returns undefined', async () => {
// Source doc representing the persisted ydoc state.
const source = TiptapTransformer.toYdoc(
doc('DB CONTENT'),
'default',
tiptapExtensions,
);
const dbState = Buffer.from(Y.encodeStateAsUpdate(source));
pageRepo.findById.mockResolvedValue({ id: PAGE_ID, ydoc: dbState });
// The hook target is a fresh empty doc (as hocuspocus supplies on cold load).
const target = freshDoc();
const result = await load(target);
// Return undefined so hocuspocus keeps `target` as-is (no second merge).
expect(result).toBeUndefined();
// The hook document now carries the DB content.
expect(jsonOf(target)).toEqual(jsonOf(source));
});
it('json→ydoc branch: converts page.content into the hook doc and returns undefined', async () => {
pageRepo.findById.mockResolvedValue({
id: PAGE_ID,
ydoc: null,
content: doc('JSON CONTENT'),
});
const target = freshDoc();
const result = await load(target);
// Returning undefined is what keeps hocuspocus from re-encoding+applying the
// state a second time (the old code returned the doc, forcing that extra
// encode). We assert the observable contract here — the return value and the
// resulting content — rather than counting internal encode calls, which is
// brittle: toYdoc and the `expected` build below both encode too.
expect(result).toBeUndefined();
// The converted content landed in the hook document.
const expected = TiptapTransformer.toYdoc(
doc('JSON CONTENT'),
'default',
tiptapExtensions,
);
expect(jsonOf(target)).toEqual(jsonOf(expected));
});
it('live doc already non-empty: early return, no DB read', async () => {
// A hocuspocus Document carrying live content (isEmpty('default') === false).
const target = freshDoc();
const live = TiptapTransformer.toYdoc(
doc('LIVE'),
'default',
tiptapExtensions,
);
Y.applyUpdate(target, Y.encodeStateAsUpdate(live));
const result = await load(target);
expect(result).toBeUndefined();
expect(pageRepo.findById).not.toHaveBeenCalled();
});
it('no persisted state: leaves the fresh empty doc untouched, returns undefined', async () => {
pageRepo.findById.mockResolvedValue({ id: PAGE_ID, ydoc: null, content: null });
const target = freshDoc();
const result = await load(target);
expect(result).toBeUndefined();
expect(target.isEmpty('default')).toBe(true);
});
});
@@ -171,15 +171,21 @@ export class PersistenceExtension implements Extension {
return;
}
// #401 fix 2 — apply the DB state DIRECTLY into the hook's target document
// (`document` === `data.document`) and return undefined. When onLoadDocument
// returns undefined, hocuspocus keeps the mutated hook document as-is; only
// when the hook RETURNS a Y.Doc does hocuspocus re-`applyUpdate(document,
// encodeStateAsUpdate(returned))` — a second full encode+apply of the whole
// (e.g. 315KB) state on every cold load. Mutating in place performs a single
// apply and avoids the throwaway `new Y.Doc()` allocation.
if (page.ydoc) {
this.logger.debug(`ydoc loaded from db: ${pageId}`);
const doc = new Y.Doc();
const dbState = new Uint8Array(page.ydoc);
Y.applyUpdate(doc, dbState);
Y.applyUpdate(document, dbState);
observeCollabLoad(dbState.length, (performance.now() - startedAt) / 1000);
return doc;
return;
}
// if no ydoc state in db convert json in page.content to Ydoc.
@@ -192,18 +198,23 @@ export class PersistenceExtension implements Extension {
tiptapExtensions,
);
// Reuse this single encode for the size label (do NOT add a second one).
// Encode the converted doc ONCE, reuse the bytes for both the size label
// and the single apply into the hook document (previously this encode's
// result was returned and hocuspocus re-encoded+applied it a second time).
const encoded = Y.encodeStateAsUpdate(ydoc);
Y.applyUpdate(document, encoded);
observeCollabLoad(
encoded.byteLength,
(performance.now() - startedAt) / 1000,
);
return ydoc;
return;
}
// No persisted state: the hook document is already a fresh empty Y.Doc, so
// leave it untouched and return undefined (no re-encode of an empty doc).
this.logger.debug(`creating fresh ydoc: ${pageId}`);
observeCollabLoad(0, (performance.now() - startedAt) / 1000);
return new Y.Doc();
return;
}
async onStoreDocument(data: onStoreDocumentPayload) {
+2 -1
View File
@@ -97,7 +97,8 @@
"patchedDependencies": {
"scimmy@1.3.5": "patches/scimmy@1.3.5.patch",
"yjs@13.6.30": "patches/yjs@13.6.30.patch",
"ai@6.0.134": "patches/ai@6.0.134.patch"
"ai@6.0.134": "patches/ai@6.0.134.patch",
"@hocuspocus/server@3.4.4": "patches/@hocuspocus__server@3.4.4.patch"
},
"overrides": {
"prosemirror-changeset": "2.4.0",
+16 -118
View File
@@ -40,7 +40,6 @@ import {
deleteNodeById,
assertUnambiguousMatch,
insertNodeRelative,
blockPlainText,
buildOutline,
getNodeByRef,
readTable,
@@ -60,12 +59,10 @@ import { getCollabToken, performLogin } from "./lib/auth-utils.js";
import { diffDocs, summarizeChange } from "./lib/diff.js";
import {
applyAnchorInDoc,
canAnchorInDoc,
countAnchorMatches,
getAnchoredText,
resolveAnchorSelection,
normalizeForMatch,
} from "./lib/comment-anchor.js";
import { closestBlockHint } from "./lib/text-normalize.js";
import {
blockText,
walk,
@@ -2477,64 +2474,6 @@ export class DocmostClient {
};
}
/** Plain text of each TOP-LEVEL block of `doc`, for anchor-failure hints. */
private topLevelBlockTexts(doc: any): string[] {
const content = doc && Array.isArray(doc.content) ? doc.content : [];
return content
.map((b: any) => blockPlainText(b))
.filter((t: string) => t.length > 0);
}
/**
* True when per-block anchoring failed but the (normalized) selection DOES
* appear in the blocks' joined plain text — i.e. it straddles a block
* boundary. Blocks are joined with a newline (collapsed to one space by
* normalizeForMatch) so a selection whose parts are separated by a paragraph
* break still matches. Callers only reach here after single-block anchoring
* (incl. the markdown-strip fallback) has already failed.
*/
private selectionSpansMultipleBlocks(
blockTexts: string[],
selection: string,
): boolean {
const normSel = normalizeForMatch(selection).norm.trim();
if (normSel.length === 0) return false;
const joined = normalizeForMatch(blockTexts.join("\n")).norm;
return joined.indexOf(normSel) !== -1;
}
/**
* Build the actionable error for a create_comment anchor MISS, porting
* edit_page_text's self-correction affordances: an explicit "spans multiple
* blocks" message when the selection straddles a block boundary, otherwise a
* "closest block text" hint quoting the block that holds the selection's
* longest token. `live` switches the wording between the pre-check (reading the
* persisted page) and the post-create live-anchor failure (which rolls back).
*/
private anchorNotFoundError(
doc: any,
selection: string,
live: boolean,
): Error {
const blockTexts = this.topLevelBlockTexts(doc);
const rolled = live ? " The comment was rolled back." : "";
if (this.selectionSpansMultipleBlocks(blockTexts, selection)) {
return new Error(
"create_comment: the selection spans multiple blocks; anchor on a " +
"contiguous fragment within a SINGLE paragraph/block (<=250 chars)." +
rolled,
);
}
const where = live ? "in the live document" : "in the page";
return new Error(
`create_comment: could not find the selection text ${where} to anchor ` +
"the comment. Provide the EXACT contiguous text from a single " +
"paragraph/block (<=250 chars)." +
closestBlockHint(blockTexts, selection) +
rolled,
);
}
/**
* Create an inline comment anchored to its `selection` text, or a reply.
*
@@ -2596,10 +2535,6 @@ export class DocmostClient {
// Captured in the pre-check below (which already reads the page) and used as
// payload.selection. Ordinary comments keep sending the raw agent selection.
let anchoredSelection: string | null = null;
// Set when the anchor matched only after stripping markdown from the
// selection (the strip fallback); surfaced as a soft warning like
// edit_page_text does, so a stale-markdown selection is flagged.
let anchorNormalized = false;
// For a top-level comment, fail BEFORE creating anything when the selection
// is not present in the persisted document — this avoids leaving an orphan
@@ -2615,7 +2550,10 @@ export class DocmostClient {
// rejected BEFORE creating the comment.
const matches = countAnchorMatches(page.content, selection);
if (matches === 0) {
throw this.anchorNotFoundError(page.content, selection, false);
throw new Error(
"create_comment: could not find the selection text in the page to anchor the comment. " +
"Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
);
}
if (matches >= 2) {
throw new Error(
@@ -2629,27 +2567,18 @@ export class DocmostClient {
// null despite countAnchorMatches===1 (shouldn't happen), fall back to
// the raw agent selection below rather than crash.
anchoredSelection = getAnchoredText(page.content, selection);
anchorNormalized = resolveAnchorSelection(
page.content,
selection,
).normalized;
} else {
const resolved = resolveAnchorSelection(page.content, selection);
if (!resolved.found) {
throw this.anchorNotFoundError(page.content, selection, false);
}
anchorNormalized = resolved.normalized;
} else if (!canAnchorInDoc(page.content, selection)) {
throw new Error(
"create_comment: could not find the selection text in the page to anchor the comment. " +
"Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
);
}
} catch (e) {
// Rethrow our own "not found"/"ambiguous"/"spans multiple blocks" errors;
// swallow read/network errors so the live anchor step can still try (and
// enforce) anchoring.
// Rethrow our own "not found"/"ambiguous" errors; swallow read/network
// errors so the live anchor step can still try (and enforce) anchoring.
if (
e instanceof Error &&
(e.message.startsWith("create_comment: could not find the selection") ||
e.message.startsWith(
"create_comment: the selection spans multiple blocks",
) ||
e.message.startsWith(
"create_comment: the suggestion's selection is ambiguous",
))
@@ -2721,10 +2650,6 @@ export class DocmostClient {
// Set inside the transform when a suggestion's live anchor is ambiguous
// (>=2 occurrences), so the rollback path can surface the right error.
let ambiguousInLiveDoc = false;
// Captured inside the transform on a not-found abort, so the rollback path
// can surface the closest-block / spans-multiple-blocks hint built from the
// LIVE document (the pre-check page is not in scope there).
let liveNotFoundError: Error | null = null;
try {
const collabToken = await this.getCollabTokenWithReauth();
// Open the collab doc by the canonical UUID, never the slugId (#260). The
@@ -2752,13 +2677,6 @@ export class DocmostClient {
const liveCount = countAnchorMatches(doc, selection as string);
if (liveCount !== 1) {
ambiguousInLiveDoc = liveCount >= 2;
if (liveCount === 0) {
liveNotFoundError = this.anchorNotFoundError(
doc,
selection as string,
true,
);
}
return null;
}
}
@@ -2768,11 +2686,6 @@ export class DocmostClient {
}
// Selection text not found in the LIVE document: abort the write. The
// rollback + throw below turns this into a hard error.
liveNotFoundError = this.anchorNotFoundError(
doc,
selection as string,
true,
);
return null;
},
);
@@ -2789,28 +2702,13 @@ export class DocmostClient {
// suggestion, was ambiguous) in the live document. Roll back the comment
// and surface a hard error.
await this.safeDeleteComment(newCommentId);
if (ambiguousInLiveDoc) {
throw new Error(
"create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique.",
);
}
throw (
liveNotFoundError ??
new Error(
"create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
)
throw new Error(
ambiguousInLiveDoc
? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique."
: "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
);
}
// Soft warning (like edit_page_text): the selection only matched after
// stripping markdown, so the caller likely quoted a styled fragment.
if (anchorNormalized) {
result.warning =
"The selection matched only after stripping markdown syntax; the comment " +
"was anchored on the document's plain text. Copy the selection verbatim " +
"from get_page / search_in_page output to avoid this.";
}
result.anchored = true;
return result;
}
+10 -84
View File
@@ -17,23 +17,8 @@
* comparing and match across maximal runs of consecutive text nodes within a
* single block, while mapping every normalized character back to its raw index
* so the mark lands on the exact original characters.
*
* MARKDOWN-STRIP FALLBACK: when the agent copies a selection that still carries
* inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not
* match the document's plain text. Exactly like edit_page_text's json-edit
* fallback, we first try the verbatim selection and, ONLY if it anchors nowhere
* in the whole document, retry with `stripInlineMarkdown` applied. `canAnchorInDoc`,
* `getAnchoredText` and `applyAnchorInDoc` share this decision via
* `resolveAnchorSelection`. `countAnchorMatches` keeps its OWN parallel exact-wins
* implementation (it needs a raw match COUNT, not a single resolved locator), kept
* deliberately in sync with `resolveAnchorSelection`: raw match ⇒ use raw, else fall
* back to the stripped count. All four therefore agree on which locator matched —
* the suggestion-uniqueness gate depends on count and can/get never disagreeing, so
* these two exact-wins implementations MUST stay in sync if either is changed.
*/
import { stripInlineMarkdown } from "./text-normalize.js";
/** Typographic double-quote variants mapped to ASCII `"`. */
const DOUBLE_QUOTES = "«»„“”‟〝〞"";
/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
@@ -229,17 +214,15 @@ function reconstructRawText(blockContent: any[], match: AnchorMatch): string {
* un-appliable (spurious 409).
*/
export function getAnchoredText(doc: any, selection: string): string | null {
const { selection: effective, found } = resolveAnchorSelection(doc, selection);
if (!found) return null;
const visit = (node: any, depth: number): string | null => {
if (depth > MAX_DEPTH || !node || typeof node !== "object") return null;
if (!Array.isArray(node.content)) return null;
const match = findAnchorInBlock(node.content, effective);
const match = findAnchorInBlock(node.content, selection);
if (match) return reconstructRawText(node.content, match);
for (const child of node.content) {
if (child && typeof child === "object" && Array.isArray(child.content)) {
const foundText = visit(child, depth + 1);
if (foundText !== null) return foundText;
const found = visit(child, depth + 1);
if (found !== null) return found;
}
}
return null;
@@ -248,11 +231,12 @@ export function getAnchoredText(doc: any, selection: string): string | null {
}
/**
* RAW (no markdown-strip fallback) depth-first check that `selection` anchors
* somewhere in `doc`. This is the primitive `resolveAnchorSelection` builds on;
* public callers should use `canAnchorInDoc`, which adds the strip fallback.
* Depth-first, document-order check for whether `selection` can be anchored
* anywhere in `doc`. At each node with an array `content`, first try to match
* within that node's own content, then recurse into children that themselves
* have a `content` array.
*/
function rawCanAnchorInDoc(doc: any, selection: string): boolean {
export function canAnchorInDoc(doc: any, selection: string): boolean {
const visit = (node: any, depth: number): boolean => {
if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
if (!Array.isArray(node.content)) return false;
@@ -267,43 +251,6 @@ function rawCanAnchorInDoc(doc: any, selection: string): boolean {
return visit(doc, 0);
}
/**
* Decide the locator that ACTUALLY anchors `selection` in `doc`, applying the
* markdown-strip fallback once (so every public entry point agrees):
* - EXACT WINS: if the verbatim selection anchors anywhere, use it as-is.
* - FALLBACK: only if the verbatim selection anchors nowhere, and the
* markdown-stripped form differs and DOES anchor, use the stripped form and
* flag `normalized` so callers can surface a soft warning.
* - otherwise `found` is false and `selection` is returned unchanged.
*
* The stripped form is used ONLY to LOCATE the anchor; getAnchoredText still
* reconstructs and stores the RAW document substring, so the strip never leaks
* into what gets persisted.
*/
export function resolveAnchorSelection(
doc: any,
selection: string,
): { selection: string; found: boolean; normalized: boolean } {
if (rawCanAnchorInDoc(doc, selection)) {
return { selection, found: true, normalized: false };
}
const stripped = stripInlineMarkdown(selection);
if (stripped !== selection && rawCanAnchorInDoc(doc, stripped)) {
return { selection: stripped, found: true, normalized: true };
}
return { selection, found: false, normalized: false };
}
/**
* Depth-first, document-order check for whether `selection` can be anchored
* anywhere in `doc` (with the markdown-strip fallback). At each node with an
* array `content`, first try to match within that node's own content, then
* recurse into children that themselves have a `content` array.
*/
export function canAnchorInDoc(doc: any, selection: string): boolean {
return resolveAnchorSelection(doc, selection).found;
}
/**
* Split the matched text nodes and splice the comment mark across the range.
* `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text
@@ -368,7 +315,7 @@ function spliceCommentMark(
* not use this. (Note: counts OCCURRENCES, not just matching blocks, so two
* occurrences inside one block are correctly reported as 2.)
*/
function rawCountAnchorMatches(doc: any, selection: string): number {
export function countAnchorMatches(doc: any, selection: string): number {
const normSel = normalizeForMatch(selection).norm.trim();
if (normSel.length === 0) return 0;
@@ -422,25 +369,6 @@ function rawCountAnchorMatches(doc: any, selection: string): number {
return total;
}
/**
* Uniqueness gate for suggestions, with the SAME markdown-strip fallback as the
* other entry points so count never disagrees with can/get/apply. EXACT WINS: if
* the verbatim selection occurs at all, return its raw occurrence count (so a
* selection that is unique raw stays unique — the fallback never runs and cannot
* introduce a spurious second match). Only when the verbatim selection is absent
* do we count occurrences of the markdown-stripped form.
*/
export function countAnchorMatches(doc: any, selection: string): number {
const raw = rawCountAnchorMatches(doc, selection);
if (raw > 0) return raw;
const stripped = stripInlineMarkdown(selection);
if (stripped !== selection) {
const strippedCount = rawCountAnchorMatches(doc, stripped);
if (strippedCount > 0) return strippedCount;
}
return 0;
}
/**
* Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block
* whose content matches `selection`, splice the comment mark across the matched
@@ -452,12 +380,10 @@ export function applyAnchorInDoc(
selection: string,
commentId: string,
): boolean {
const { selection: effective, found } = resolveAnchorSelection(doc, selection);
if (!found) return false;
const visit = (node: any, depth: number): boolean => {
if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
if (!Array.isArray(node.content)) return false;
const match = findAnchorInBlock(node.content, effective);
const match = findAnchorInBlock(node.content, selection);
if (match) {
spliceCommentMark(node.content, match, commentId);
return true;
+24 -8
View File
@@ -12,11 +12,7 @@
* re-import for small wording fixes.
*/
import {
stripInlineMarkdown,
stripBalancedWrappers,
closestBlockHint,
} from "./text-normalize.js";
import { stripInlineMarkdown, stripBalancedWrappers } from "./text-normalize.js";
export interface TextEdit {
find: string;
@@ -385,9 +381,29 @@ export function applyTextEdits(
} else {
// Append a bounded "closest text" hint: find the FIRST block that
// contains the longest whitespace-delimited token (>= 3 chars) of the
// (stripped, then raw) locator, and quote that block's plain text. Shared
// with create_comment via closestBlockHint so both give the same hint.
reason = "text not found in the document." + closestBlockHint(blockPlain, edit.find);
// (stripped, then raw) locator, and quote that block's plain text.
reason = "text not found in the document.";
const tokenSource = stripped.length > 0 ? stripped : edit.find;
const longestToken = tokenSource
.split(/\s+/)
.filter((t) => t.length >= 3)
.sort((a, b) => b.length - a.length)[0];
if (longestToken) {
const hitBlock = blockPlain.find((plain) =>
plain.includes(longestToken),
);
if (hitBlock) {
// Truncate by code point (spread iterates by code point) so a
// surrogate pair is never split; append the ellipsis only when the
// text was actually longer than the limit.
const points = [...hitBlock];
const snippet =
points.length > 120
? points.slice(0, 120).join("") + "…"
: hitBlock;
reason += ` Closest block text: "${snippet}".`;
}
}
}
failed.push({ find: edit.find, reason });
continue;
-34
View File
@@ -114,37 +114,3 @@ export function stripInlineMarkdown(s: string): string {
return out;
}
/**
* Build a bounded "closest text" hint for an anchor/find MISS, shared by
* edit_page_text (json-edit) and create_comment (client) so both surface the
* same self-correction affordance.
*
* Take the longest whitespace-delimited token (>= 3 chars) of the locator
* (markdown-stripped first, so `**bold**` contributes `bold`), find the FIRST
* of `blockTexts` that contains it, and return ` Closest block text: "…".` with
* the block quoted (truncated to 120 code points + ellipsis). Returns "" when
* no token qualifies or no block contains it, so the caller can append it
* unconditionally.
*/
export function closestBlockHint(
blockTexts: string[],
locator: string,
): string {
if (typeof locator !== "string" || locator.length === 0) return "";
const stripped = stripInlineMarkdown(locator);
const tokenSource = stripped.length > 0 ? stripped : locator;
const longestToken = tokenSource
.split(/\s+/)
.filter((t) => t.length >= 3)
.sort((a, b) => b.length - a.length)[0];
if (!longestToken) return "";
const hitBlock = blockTexts.find((plain) => plain.includes(longestToken));
if (!hitBlock) return "";
// Truncate by code point (spread iterates by code point) so a surrogate pair
// is never split; append the ellipsis only when the text was actually longer.
const points = [...hitBlock];
const snippet =
points.length > 120 ? points.slice(0, 120).join("") + "…" : hitBlock;
return ` Closest block text: "${snippet}".`;
}
+3 -7
View File
@@ -771,13 +771,9 @@ export const SHARED_TOOL_SPECS = {
'The comment is anchored inline to the given exact `selection` text ' +
'(which gets highlighted); page-level comments are NOT supported. A ' +
'new top-level comment REQUIRES a `selection`. Replies inherit the ' +
"parent's anchor and take no selection. Always COPY the `selection` " +
'VERBATIM from get_page / search_in_page output — do NOT quote it from ' +
'memory (stale-memory quoting is the top cause of anchor misses). If the ' +
'call fails with a "selection not found" error, the error quotes the ' +
"closest block text (or says the selection spans multiple blocks); retry " +
"with a corrected EXACT selection copied verbatim from a single " +
'paragraph/block. You may also attach a ' +
"parent's anchor and take no selection. If the call fails with a " +
'"selection not found" error, retry with a corrected EXACT selection ' +
'copied verbatim from a single paragraph/block. You may also attach a ' +
'`suggestedText` proposing a replacement for the `selection` (a human ' +
'applies it from the UI); when set, the `selection` must occur exactly ' +
'once in the page. Reversible via the comment UI.',
@@ -548,94 +548,3 @@ test("suggestedText: the stored selection is the doc's RAW typographic substring
);
assert.equal(createPayload.suggestedText, "goodbye");
});
// -----------------------------------------------------------------------------
// 8) #408: a not-found selection error QUOTES the closest block text so the
// model can self-correct instead of blind-retrying.
// -----------------------------------------------------------------------------
test("a not-found selection error includes a 'Closest block text' hint", async () => {
let createCalls = 0;
const { baseURL } = await spawn(async (req, res) => {
await readBody(req);
if (req.url === "/api/auth/login") {
sendJson(res, 200, { success: true }, {
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
});
return;
}
if (req.url === "/api/pages/info") {
sendJson(res, 200, {
data: {
id: "page-1",
content: {
type: "doc",
content: [
{ type: "paragraph", content: [{ type: "text", text: "The quick brown fox jumps" }] },
],
},
},
});
return;
}
if (req.url === "/api/comments/create") {
createCalls++;
sendJson(res, 200, { data: { id: "should-not-happen" } });
return;
}
sendJson(res, 404, { message: "not found" });
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() => client.createComment("page-1", "body", "inline", "quick brown cat"),
/Closest block text: "The quick brown fox jumps"/,
"a not-found selection must quote the closest block text",
);
assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
});
// -----------------------------------------------------------------------------
// 9) #408: a selection that straddles two blocks gets the explicit
// "spans multiple blocks" message instead of a bare not-found.
// -----------------------------------------------------------------------------
test("a selection spanning multiple blocks gets the explicit spans-multiple-blocks message", async () => {
let createCalls = 0;
const { baseURL } = await spawn(async (req, res) => {
await readBody(req);
if (req.url === "/api/auth/login") {
sendJson(res, 200, { success: true }, {
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
});
return;
}
if (req.url === "/api/pages/info") {
sendJson(res, 200, {
data: {
id: "page-1",
content: {
type: "doc",
content: [
{ type: "paragraph", content: [{ type: "text", text: "the quick brown" }] },
{ type: "paragraph", content: [{ type: "text", text: "fox jumps over" }] },
],
},
},
});
return;
}
if (req.url === "/api/comments/create") {
createCalls++;
sendJson(res, 200, { data: { id: "should-not-happen" } });
return;
}
sendJson(res, 404, { message: "not found" });
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() => client.createComment("page-1", "body", "inline", "brown fox"),
/spans multiple blocks/,
"a cross-block selection must report the spans-multiple-blocks hint",
);
assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
});
@@ -8,7 +8,6 @@ import {
applyAnchorInDoc,
countAnchorMatches,
getAnchoredText,
resolveAnchorSelection,
} from "../../build/lib/comment-anchor.js";
const COMMENT_ID = "cmt-123";
@@ -309,70 +308,3 @@ test("getAnchoredText returns null when the selection does not anchor", () => {
const doc = paragraphDoc([{ type: "text", text: "hello world" }]);
assert.equal(getAnchoredText(doc, "not present"), null);
});
// ---------------------------------------------------------------------------
// #408 MARKDOWN-STRIP FALLBACK. A selection copied with inline markdown still
// carries `**`/`` ` ``/`[t](u)` markers the plain document text lacks. When the
// verbatim selection anchors nowhere, all four entry points retry with the
// markdown stripped — consistently, so the suggestion-uniqueness gate stays
// coherent — while what gets STORED remains the raw document substring.
// ---------------------------------------------------------------------------
test("a markdown-styled selection anchors against plain doc text via the strip fallback", () => {
const doc = paragraphDoc([{ type: "text", text: "a bold word here" }]);
// The agent quoted "**bold** word" from a styled view; the doc is plain text.
const sel = "**bold** word";
const resolved = resolveAnchorSelection(doc, sel);
assert.equal(resolved.found, true, "strip fallback finds the anchor");
assert.equal(resolved.normalized, true, "reports the soft-warning flag");
assert.equal(canAnchorInDoc(doc, sel), true);
assert.equal(countAnchorMatches(doc, sel), 1);
const ok = applyAnchorInDoc(doc, sel, COMMENT_ID);
assert.equal(ok, true);
const marked = doc.content[0].content.filter((p) => commentMark(p));
assert.equal(marked.map((m) => m.text).join(""), "bold word",
"the mark lands on the plain-text span");
});
test("getAnchoredText stores the RAW doc substring even when matched via the strip fallback", () => {
// Doc uses a smart apostrophe; the agent typed ASCII + markdown emphasis.
const doc = paragraphDoc([{ type: "text", text: "it’s bold now" }]);
const stored = getAnchoredText(doc, "it's **bold**");
assert.equal(stored, "it’s bold",
"stored selection is the raw document text, not the stripped/ASCII locator");
});
test("the strip fallback does not flip a raw-unique selection to ambiguous", () => {
// "config" appears twice, but the raw phrase "config value" appears once.
const doc = {
type: "doc",
content: [
{ type: "paragraph", content: [{ type: "text", text: "the config value here" }] },
{ type: "paragraph", content: [{ type: "text", text: "another config here" }] },
],
};
// Raw phrase is unique -> exactly 1, and no strip happens (nothing to strip).
assert.equal(countAnchorMatches(doc, "config value"), 1);
assert.equal(resolveAnchorSelection(doc, "config value").normalized, false);
});
test("EXACT WINS: a raw match short-circuits the strip fallback (count reflects raw)", () => {
// A literal "**" run exists raw once; its stripped form would also appear.
const doc = paragraphDoc([{ type: "text", text: "use **stars** and stars" }]);
// Raw "**stars**" occurs once -> count 1 from the verbatim locator; the
// fallback (which would find two "stars") never runs.
assert.equal(countAnchorMatches(doc, "**stars**"), 1);
assert.equal(resolveAnchorSelection(doc, "**stars**").normalized, false);
});
test("a markdown selection whose stripped form is ambiguous is counted as ambiguous", () => {
const doc = {
type: "doc",
content: [
{ type: "paragraph", content: [{ type: "text", text: "first config here" }] },
{ type: "paragraph", content: [{ type: "text", text: "second config here" }] },
],
};
// Verbatim "**config**" matches nothing; stripped "config" matches twice.
assert.equal(countAnchorMatches(doc, "**config**"), 2);
});
+62
View File
@@ -0,0 +1,62 @@
diff --git a/dist/hocuspocus-server.cjs b/dist/hocuspocus-server.cjs
index b24ff6d091c32f733089eeaa47b03f7b37cf5964..f003af304fc751b7edc1aee17f3651282d70666a 100644
--- a/dist/hocuspocus-server.cjs
+++ b/dist/hocuspocus-server.cjs
@@ -2426,6 +2426,26 @@ class Hocuspocus {
* Create a new document by the given request
*/
async createDocument(documentName, request, socketId, connection, context) {
+ // PATCH(gitmost #401): close the connect-vs-unload race. When the last
+ // client disconnects, storeDocumentHooks' finally schedules an unload;
+ // unloadDocument runs its beforeUnloadDocument hooks asynchronously and
+ // records an in-flight promise in `unloadingDocuments`. A NEW connection
+ // arriving in that window would otherwise fall straight through to the
+ // `documents`/`loadingDocuments` checks and could either reuse a doc that
+ // is about to be destroyed, or start loading a fresh doc concurrently
+ // with the destroy. Awaiting the in-flight unload first makes the decision
+ // deterministic: once it settles, either the doc was fully unloaded
+ // (removed from `documents`, so we do a clean fresh load below) or the
+ // unload aborted because work/connections reappeared (the healthy doc is
+ // still in `documents`, so we reuse it). Either way the new connection can
+ // never hand-shake onto an orphaned, about-to-be-destroyed Document.
+ const existingUnloadingDoc = this.unloadingDocuments.get(documentName);
+ if (existingUnloadingDoc) {
+ // Wait for the in-flight unload to settle so we never hand-shake onto a dying
+ // Document. Swallow a rejected unload — fall through to a fresh load, matching
+ // pre-patch behavior (the doc is already removed from `documents` by then).
+ try { await existingUnloadingDoc; } catch { /* unload rejected — fresh load */ }
+ }
const existingLoadingDoc = this.loadingDocuments.get(documentName);
if (existingLoadingDoc) {
return existingLoadingDoc;
diff --git a/dist/hocuspocus-server.esm.js b/dist/hocuspocus-server.esm.js
index 1f4dd80244e899128e2c4e5dad8eab7cfc1cbad6..8c2411747bba27fb9486e1df81678a14e41e884e 100644
--- a/dist/hocuspocus-server.esm.js
+++ b/dist/hocuspocus-server.esm.js
@@ -2406,6 +2406,26 @@ class Hocuspocus {
* Create a new document by the given request
*/
async createDocument(documentName, request, socketId, connection, context) {
+ // PATCH(gitmost #401): close the connect-vs-unload race. When the last
+ // client disconnects, storeDocumentHooks' finally schedules an unload;
+ // unloadDocument runs its beforeUnloadDocument hooks asynchronously and
+ // records an in-flight promise in `unloadingDocuments`. A NEW connection
+ // arriving in that window would otherwise fall straight through to the
+ // `documents`/`loadingDocuments` checks and could either reuse a doc that
+ // is about to be destroyed, or start loading a fresh doc concurrently
+ // with the destroy. Awaiting the in-flight unload first makes the decision
+ // deterministic: once it settles, either the doc was fully unloaded
+ // (removed from `documents`, so we do a clean fresh load below) or the
+ // unload aborted because work/connections reappeared (the healthy doc is
+ // still in `documents`, so we reuse it). Either way the new connection can
+ // never hand-shake onto an orphaned, about-to-be-destroyed Document.
+ const existingUnloadingDoc = this.unloadingDocuments.get(documentName);
+ if (existingUnloadingDoc) {
+ // Wait for the in-flight unload to settle so we never hand-shake onto a dying
+ // Document. Swallow a rejected unload — fall through to a fresh load, matching
+ // pre-patch behavior (the doc is already removed from `documents` by then).
+ try { await existingUnloadingDoc; } catch { /* unload rejected — fresh load */ }
+ }
const existingLoadingDoc = this.loadingDocuments.get(documentName);
if (existingLoadingDoc) {
return existingLoadingDoc;
+5 -2
View File
@@ -44,6 +44,9 @@ overrides:
ip-address: 10.1.1
patchedDependencies:
'@hocuspocus/server@3.4.4':
hash: d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42
path: patches/@hocuspocus__server@3.4.4.patch
ai@6.0.134:
hash: f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9
path: patches/ai@6.0.134.patch
@@ -75,7 +78,7 @@ importers:
version: 3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
'@hocuspocus/server':
specifier: 3.4.4
version: 3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
version: 3.4.4(patch_hash=d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
'@hocuspocus/transformer':
specifier: 3.4.4
version: 3.4.4(@tiptap/core@3.20.4(@tiptap/pm@3.20.4))(@tiptap/pm@3.20.4)(y-prosemirror@1.3.7(prosemirror-model@1.25.1)(prosemirror-state@1.4.3)(prosemirror-view@1.40.0)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
@@ -13055,7 +13058,7 @@ snapshots:
- bufferutil
- utf-8-validate
'@hocuspocus/server@3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))':
'@hocuspocus/server@3.4.4(patch_hash=d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))':
dependencies:
'@hocuspocus/common': 3.4.4
async-lock: 1.4.1