Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 agent_coder
|
72c2d1687e | ||
|
agent_coder
|
c9293e316b |
@@ -1,140 +0,0 @@
|
||||
/**
|
||||
* 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);
|
||||
});
|
||||
});
|
||||
@@ -1,130 +0,0 @@
|
||||
/**
|
||||
* 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,21 +171,15 @@ 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(document, dbState);
|
||||
Y.applyUpdate(doc, dbState);
|
||||
observeCollabLoad(dbState.length, (performance.now() - startedAt) / 1000);
|
||||
return;
|
||||
return doc;
|
||||
}
|
||||
|
||||
// if no ydoc state in db convert json in page.content to Ydoc.
|
||||
@@ -198,23 +192,18 @@ export class PersistenceExtension implements Extension {
|
||||
tiptapExtensions,
|
||||
);
|
||||
|
||||
// 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).
|
||||
// Reuse this single encode for the size label (do NOT add a second one).
|
||||
const encoded = Y.encodeStateAsUpdate(ydoc);
|
||||
Y.applyUpdate(document, encoded);
|
||||
observeCollabLoad(
|
||||
encoded.byteLength,
|
||||
(performance.now() - startedAt) / 1000,
|
||||
);
|
||||
return;
|
||||
return ydoc;
|
||||
}
|
||||
|
||||
// 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;
|
||||
return new Y.Doc();
|
||||
}
|
||||
|
||||
async onStoreDocument(data: onStoreDocumentPayload) {
|
||||
|
||||
+1
-2
@@ -97,8 +97,7 @@
|
||||
"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",
|
||||
"@hocuspocus/server@3.4.4": "patches/@hocuspocus__server@3.4.4.patch"
|
||||
"ai@6.0.134": "patches/ai@6.0.134.patch"
|
||||
},
|
||||
"overrides": {
|
||||
"prosemirror-changeset": "2.4.0",
|
||||
|
||||
+118
-16
@@ -40,6 +40,7 @@ import {
|
||||
deleteNodeById,
|
||||
assertUnambiguousMatch,
|
||||
insertNodeRelative,
|
||||
blockPlainText,
|
||||
buildOutline,
|
||||
getNodeByRef,
|
||||
readTable,
|
||||
@@ -59,10 +60,12 @@ 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,
|
||||
@@ -2474,6 +2477,64 @@ 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.
|
||||
*
|
||||
@@ -2535,6 +2596,10 @@ 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
|
||||
@@ -2550,10 +2615,7 @@ export class DocmostClient {
|
||||
// rejected BEFORE creating the comment.
|
||||
const matches = countAnchorMatches(page.content, selection);
|
||||
if (matches === 0) {
|
||||
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).",
|
||||
);
|
||||
throw this.anchorNotFoundError(page.content, selection, false);
|
||||
}
|
||||
if (matches >= 2) {
|
||||
throw new Error(
|
||||
@@ -2567,18 +2629,27 @@ 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);
|
||||
} 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).",
|
||||
);
|
||||
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;
|
||||
}
|
||||
} catch (e) {
|
||||
// Rethrow our own "not found"/"ambiguous" errors; swallow read/network
|
||||
// errors so the live anchor step can still try (and enforce) anchoring.
|
||||
// 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.
|
||||
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",
|
||||
))
|
||||
@@ -2650,6 +2721,10 @@ 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
|
||||
@@ -2677,6 +2752,13 @@ 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;
|
||||
}
|
||||
}
|
||||
@@ -2686,6 +2768,11 @@ 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;
|
||||
},
|
||||
);
|
||||
@@ -2702,13 +2789,28 @@ export class DocmostClient {
|
||||
// suggestion, was ambiguous) in the live document. Roll back the comment
|
||||
// and surface a hard error.
|
||||
await this.safeDeleteComment(newCommentId);
|
||||
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",
|
||||
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",
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
@@ -17,8 +17,23 @@
|
||||
* 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 `'`. */
|
||||
@@ -214,15 +229,17 @@ 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, selection);
|
||||
const match = findAnchorInBlock(node.content, effective);
|
||||
if (match) return reconstructRawText(node.content, match);
|
||||
for (const child of node.content) {
|
||||
if (child && typeof child === "object" && Array.isArray(child.content)) {
|
||||
const found = visit(child, depth + 1);
|
||||
if (found !== null) return found;
|
||||
const foundText = visit(child, depth + 1);
|
||||
if (foundText !== null) return foundText;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
@@ -231,12 +248,11 @@ export function getAnchoredText(doc: any, selection: string): string | null {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* 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.
|
||||
*/
|
||||
export function canAnchorInDoc(doc: any, selection: string): boolean {
|
||||
function rawCanAnchorInDoc(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;
|
||||
@@ -251,6 +267,43 @@ export function canAnchorInDoc(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
|
||||
@@ -315,7 +368,7 @@ function spliceCommentMark(
|
||||
* not use this. (Note: counts OCCURRENCES, not just matching blocks, so two
|
||||
* occurrences inside one block are correctly reported as 2.)
|
||||
*/
|
||||
export function countAnchorMatches(doc: any, selection: string): number {
|
||||
function rawCountAnchorMatches(doc: any, selection: string): number {
|
||||
const normSel = normalizeForMatch(selection).norm.trim();
|
||||
if (normSel.length === 0) return 0;
|
||||
|
||||
@@ -369,6 +422,25 @@ export function countAnchorMatches(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
|
||||
@@ -380,10 +452,12 @@ 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, selection);
|
||||
const match = findAnchorInBlock(node.content, effective);
|
||||
if (match) {
|
||||
spliceCommentMark(node.content, match, commentId);
|
||||
return true;
|
||||
|
||||
@@ -12,7 +12,11 @@
|
||||
* re-import for small wording fixes.
|
||||
*/
|
||||
|
||||
import { stripInlineMarkdown, stripBalancedWrappers } from "./text-normalize.js";
|
||||
import {
|
||||
stripInlineMarkdown,
|
||||
stripBalancedWrappers,
|
||||
closestBlockHint,
|
||||
} from "./text-normalize.js";
|
||||
|
||||
export interface TextEdit {
|
||||
find: string;
|
||||
@@ -381,29 +385,9 @@ 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.
|
||||
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}".`;
|
||||
}
|
||||
}
|
||||
// (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);
|
||||
}
|
||||
failed.push({ find: edit.find, reason });
|
||||
continue;
|
||||
|
||||
@@ -114,3 +114,37 @@ 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}".`;
|
||||
}
|
||||
|
||||
@@ -771,9 +771,13 @@ 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. 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 ' +
|
||||
"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 ' +
|
||||
'`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,3 +548,94 @@ 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,6 +8,7 @@ import {
|
||||
applyAnchorInDoc,
|
||||
countAnchorMatches,
|
||||
getAnchoredText,
|
||||
resolveAnchorSelection,
|
||||
} from "../../build/lib/comment-anchor.js";
|
||||
|
||||
const COMMENT_ID = "cmt-123";
|
||||
@@ -308,3 +309,70 @@ 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);
|
||||
});
|
||||
|
||||
@@ -1,62 +0,0 @@
|
||||
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;
|
||||
Generated
+2
-5
@@ -44,9 +44,6 @@ 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
|
||||
@@ -78,7 +75,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(patch_hash=d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
|
||||
version: 3.4.4(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))
|
||||
@@ -13058,7 +13055,7 @@ snapshots:
|
||||
- bufferutil
|
||||
- utf-8-validate
|
||||
|
||||
'@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))':
|
||||
'@hocuspocus/server@3.4.4(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
|
||||
|
||||
Reference in New Issue
Block a user