Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 agent_coder
|
0f5f048ca2 |
@@ -28,10 +28,7 @@ const h = vi.hoisted(() => ({
|
||||
body: Record<string, unknown>;
|
||||
}) => { body: Record<string, unknown> };
|
||||
prepareReconnectToStreamRequest?: () => { api?: string };
|
||||
fetch?: (
|
||||
input: unknown,
|
||||
init?: { method?: string; body?: unknown },
|
||||
) => Promise<unknown>;
|
||||
fetch?: (input: unknown, init?: { method?: string }) => Promise<unknown>;
|
||||
},
|
||||
},
|
||||
}));
|
||||
@@ -203,244 +200,6 @@ describe("ChatThread — send now (#198)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #396: in autonomous mode a live sendNow must additionally request the
|
||||
// AUTHORITATIVE server stop of the detached run (a local abort is only a client
|
||||
// disconnect the server ignores) and arm a bounded 409 retry so the re-POST
|
||||
// converges once the one-active-run slot frees. Legacy mode is unchanged.
|
||||
describe("ChatThread — send now server-stop + supersede retry (#396)", () => {
|
||||
beforeEach(resetState);
|
||||
afterEach(cleanup);
|
||||
|
||||
// A settled assistant tail => no mount resume (attemptResumeRef false), so the
|
||||
// "Send now" button is visible for the NEW local streaming turn while
|
||||
// autonomous runs are enabled.
|
||||
const settledTail = () => [
|
||||
row("u1", "user", undefined, "hi"),
|
||||
row("a1", "assistant", "succeeded", "done"),
|
||||
];
|
||||
|
||||
it("autonomous: sendNow during a live stream calls onServerStop with the chat id", () => {
|
||||
const { onServerStop } = renderThread({
|
||||
autonomousRunsEnabled: true,
|
||||
initialRows: settledTail(),
|
||||
});
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
|
||||
expect(h.state.stop).toHaveBeenCalledTimes(1);
|
||||
expect(onServerStop).toHaveBeenCalledWith("c1");
|
||||
});
|
||||
|
||||
it("legacy (autonomous off): sendNow does NOT call onServerStop and does NOT retry the send", async () => {
|
||||
const { onServerStop } = renderThread({
|
||||
autonomousRunsEnabled: false,
|
||||
initialRows: settledTail(),
|
||||
});
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
expect(onServerStop).not.toHaveBeenCalled();
|
||||
|
||||
// The supersede retry must NOT be armed: a POST that 409s is returned as-is
|
||||
// (single fetch, no retry).
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("armed supersede send retries 409 A_RUN_ALREADY_ACTIVE and succeeds once the slot frees", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
// Arm the retry by performing a live sendNow (autonomous branch sets the ref).
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
// First POST: the old detached run still holds the slot -> 409.
|
||||
.mockResolvedValueOnce(
|
||||
new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
)
|
||||
// Retry: the server stop settled the old run -> 200.
|
||||
.mockResolvedValueOnce(new Response("ok", { status: 200 }));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it("supersede retry is one-shot: a later send (ref cleared) does NOT retry a 409", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now")); // arms the one-shot
|
||||
|
||||
// First armed send: immediately succeeds, consuming the arm.
|
||||
let fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue(new Response("ok", { status: 200 }));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
});
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
// A subsequent send is NOT armed -> a 409 is returned as-is (no retry).
|
||||
fetchMock = vi.fn().mockResolvedValue(
|
||||
new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("supersede retry is bounded: exhaustion surfaces the 409 error", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
|
||||
// Every attempt 409s -> after 4 attempts the last 409 surfaces.
|
||||
const fetchMock = vi.fn().mockResolvedValue(
|
||||
new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
// 4 attempts total (1 immediate + 3 backoff retries), then give up.
|
||||
expect(fetchMock).toHaveBeenCalledTimes(4);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("armed supersede send does NOT retry a non-409 status", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue(new Response("boom", { status: 500 }));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).toBe(500);
|
||||
});
|
||||
|
||||
// Strand-path regression: sendNow arms the supersede retry, but if the promoted
|
||||
// head is removed before the abort's onFinish lands, flushNext() sends nothing
|
||||
// (returns false) and NO re-POST consumes the arm. The arm must be disarmed on
|
||||
// that no-send branch so the NEXT unrelated NORMAL send does not inherit it and
|
||||
// silently retry a genuine 409 (e.g. a legitimate two-tab conflict) 4x instead
|
||||
// of surfacing it immediately.
|
||||
it("strand-path: a stranded supersede arm (flushNext no-send) does NOT retry a later normal 409", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
// Arm the retry via a live autonomous sendNow (promotes the head + arms).
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
|
||||
// Remove the promoted head BEFORE the abort lands, so flushNext() returns
|
||||
// false (no POST) and the arm would strand without the disarm fix.
|
||||
fireEvent.click(screen.getByLabelText("Remove queued message"));
|
||||
|
||||
// The abort's onFinish now takes the flushOnAbortRef branch, calls flushNext()
|
||||
// which finds an empty queue and returns false -> the no-send disarm must run.
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: { id: "a1", role: "assistant", parts: [] },
|
||||
isAbort: true,
|
||||
isDisconnect: false,
|
||||
isError: false,
|
||||
});
|
||||
});
|
||||
// No re-POST was sent (nothing to flush).
|
||||
expect(h.state.sendMessage).not.toHaveBeenCalled();
|
||||
|
||||
// A subsequent NORMAL send that 409s must be returned as-is (exactly 1 fetch):
|
||||
// the stranded arm must NOT cause the genuine 409 to be retried.
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
new Response(JSON.stringify({ code: "A_RUN_ALREADY_ACTIVE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("armed supersede send does NOT retry a 409 with a different (non-A_RUN_ALREADY_ACTIVE) body", async () => {
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: settledTail() });
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now"));
|
||||
const fetchMock = vi.fn().mockResolvedValue(
|
||||
new Response(JSON.stringify({ code: "SOMETHING_ELSE" }), {
|
||||
status: 409,
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
let res!: Response;
|
||||
await act(async () => {
|
||||
res = (await h.state.transport!.fetch!("http://x", {
|
||||
method: "POST",
|
||||
body: "{}",
|
||||
})) as Response;
|
||||
});
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
// #388: the editor selection is snapshotted at send time and nested inside
|
||||
// openPage on the wire. The getter is read live from a ref, so each send ships a
|
||||
// fresh snapshot.
|
||||
|
||||
@@ -70,36 +70,6 @@ const RECONNECT_MAX_ATTEMPTS = 5;
|
||||
// Backoff before attempt N (1-based): 1s, 2s, 4s, 8s, 16s.
|
||||
const RECONNECT_BASE_DELAY_MS = 1000;
|
||||
|
||||
// #396: bounded retry for the "Interrupt and send now" re-send when it races the
|
||||
// authoritative server stop of the just-superseded detached run. The re-POST can
|
||||
// arrive before the old run has released the one-active-run slot, so the server
|
||||
// returns 409 A_RUN_ALREADY_ACTIVE. The server stop guarantees the slot frees, so
|
||||
// a few short backoffs converge. 4 total attempts: attempt 1 fires immediately,
|
||||
// then these are the waits BEFORE attempts 2, 3 and 4 (150ms, 300ms, 600ms). If
|
||||
// all 4 attempts 409, the last 409 surfaces (the banner) — acceptable per #396.
|
||||
const SUPERSEDE_RETRY_DELAYS_MS = [150, 300, 600];
|
||||
// The server error code that means "another run is already active for this chat".
|
||||
const A_RUN_ALREADY_ACTIVE = "A_RUN_ALREADY_ACTIVE";
|
||||
|
||||
/**
|
||||
* #396: defensively decide whether a 409 response is the one-active-run gate
|
||||
* rejection (code A_RUN_ALREADY_ACTIVE) vs. some other 409. Reads a CLONE so the
|
||||
* original response body stays intact for the caller when it is returned as-is.
|
||||
* Any parse failure or unexpected shape => false (do NOT retry).
|
||||
*/
|
||||
async function isRunAlreadyActive(response: Response): Promise<boolean> {
|
||||
try {
|
||||
const body = (await response.clone().json()) as unknown;
|
||||
return (
|
||||
typeof body === "object" &&
|
||||
body !== null &&
|
||||
(body as { code?: unknown }).code === A_RUN_ALREADY_ACTIVE
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** The page the user is currently viewing, sent as chat context. */
|
||||
export interface OpenPageContext {
|
||||
id: string;
|
||||
@@ -356,26 +326,6 @@ export default function ChatThread({
|
||||
const flushOnAbortRef = useRef(false);
|
||||
const interruptNextSendRef = useRef(false);
|
||||
|
||||
// #396: one-shot arm for the bounded 409 A_RUN_ALREADY_ACTIVE retry on the
|
||||
// "Interrupt and send now" re-send in autonomous mode. sendNow triggers the
|
||||
// authoritative server stop of the detached run, but that stop and the
|
||||
// onFinish->flushNext re-POST race: the new POST can hit the one-active-run
|
||||
// gate before the old detached run has settled, yielding a spurious 409. When
|
||||
// this ref is armed, the transport's send path retries that 409 with a short
|
||||
// bounded backoff (the server stop guarantees convergence). A normal send (ref
|
||||
// not armed) must STILL fail a 409 instantly (e.g. a genuine two-tab conflict).
|
||||
//
|
||||
// INVARIANT: sendNow arms this only to be consumed by the ONE re-POST that
|
||||
// flushNext fires from onFinish. But that re-POST does not always happen (the
|
||||
// promoted head may be gone, the finish may be a resumed turn, or the arm may
|
||||
// race a stale finish). To keep the arm strictly one-shot it is disarmed on
|
||||
// EVERY path where the paired interrupt one-shots (flushOnAbortRef /
|
||||
// interruptNextSendRef) are cleared without a POST: the transport POST branch
|
||||
// consumes it (read-and-clear), the onFinish `!flushNext()` no-send branch
|
||||
// clears it, and the isStreaming-defuse effect clears it symmetrically. So it
|
||||
// can never leak into a later, unrelated send and retry that send's genuine 409.
|
||||
const supersedeRetryRef = useRef(false);
|
||||
|
||||
// #234 F5: the user pressed Stop while streaming a BRAND-NEW chat whose server
|
||||
// chat id has not been adopted yet (the `start` chunk carrying it hadn't landed
|
||||
// when Stop was pressed). A local SSE abort alone does NOT stop the DETACHED
|
||||
@@ -432,43 +382,7 @@ export default function ChatThread({
|
||||
}`,
|
||||
}),
|
||||
fetch: async (input: RequestInfo | URL, init: RequestInit = {}) => {
|
||||
if ((init.method ?? "GET") !== "GET") {
|
||||
// Send path (POST). #396: read-and-clear the one-shot supersede arm
|
||||
// here so it is strictly scoped to THIS send. When unarmed, behave
|
||||
// exactly as before — a single fetch, a 409 surfaces instantly (a
|
||||
// genuine two-tab conflict must NOT be retried).
|
||||
const supersede = supersedeRetryRef.current;
|
||||
supersedeRetryRef.current = false;
|
||||
if (!supersede) return fetch(input, init);
|
||||
// Buffer a ReadableStream body once so each retry can replay it.
|
||||
// DefaultChatTransport sends the body as a JSON STRING (replayable as
|
||||
// is), but guard defensively in case a future SDK streams it.
|
||||
let sendInit = init;
|
||||
if (init.body instanceof ReadableStream) {
|
||||
const buffered = await new Response(init.body).arrayBuffer();
|
||||
sendInit = { ...init, body: buffered };
|
||||
}
|
||||
// Bounded retry: attempt 1 fires immediately, then wait between
|
||||
// attempts per SUPERSEDE_RETRY_DELAYS_MS. Retry ONLY on a real
|
||||
// 409 A_RUN_ALREADY_ACTIVE; any other status/body is returned as-is.
|
||||
for (let attempt = 0; ; attempt++) {
|
||||
const response = await fetch(input, sendInit);
|
||||
if (
|
||||
response.status !== 409 ||
|
||||
attempt >= SUPERSEDE_RETRY_DELAYS_MS.length ||
|
||||
!(await isRunAlreadyActive(response))
|
||||
) {
|
||||
return response;
|
||||
}
|
||||
// The old detached run has not released the one-active-run slot
|
||||
// yet; the server stop we requested guarantees it will, so back off
|
||||
// and re-POST (the 409 fired before the user message was persisted,
|
||||
// so re-POSTing is safe — no duplicate rows).
|
||||
await new Promise((r) =>
|
||||
setTimeout(r, SUPERSEDE_RETRY_DELAYS_MS[attempt]),
|
||||
);
|
||||
}
|
||||
}
|
||||
if ((init.method ?? "GET") !== "GET") return fetch(input, init); // send path untouched
|
||||
// Reconnect GET: the SDK passes no AbortSignal, so wire our own controller
|
||||
// for observer Stop / unmount abort.
|
||||
const controller = new AbortController();
|
||||
@@ -648,14 +562,9 @@ export default function ChatThread({
|
||||
setStopNotice(null);
|
||||
// If the promoted head vanished (e.g. the user removed it before the
|
||||
// abort landed) flushNext sends nothing — clear the one-shot interrupt
|
||||
// tag AND the #396 supersede arm so neither can leak onto the next
|
||||
// unrelated send (no re-POST will consume the arm here). On a real send
|
||||
// the tag is consumed by prepareSendMessagesRequest and the arm by the
|
||||
// transport POST branch, so both stay untouched then.
|
||||
if (!flushNext()) {
|
||||
interruptNextSendRef.current = false;
|
||||
supersedeRetryRef.current = false;
|
||||
}
|
||||
// tag so it can't leak onto the next unrelated send. On a real send the
|
||||
// tag is consumed by prepareSendMessagesRequest and stays untouched.
|
||||
if (!flushNext()) interruptNextSendRef.current = false;
|
||||
return;
|
||||
}
|
||||
if (isAbort || isDisconnect || isError) return;
|
||||
@@ -964,30 +873,6 @@ export default function ChatThread({
|
||||
setQueue(promoteToHead(queuedRef.current, id));
|
||||
flushOnAbortRef.current = true;
|
||||
interruptNextSendRef.current = true;
|
||||
// #396: in autonomous mode the turn is a DETACHED run — a local stop()
|
||||
// is only a client disconnect the server ignores, so the run keeps going.
|
||||
// The onFinish->flushNext re-POST would then hit the one-active-run gate
|
||||
// and get a spurious 409 A_RUN_ALREADY_ACTIVE. Mirror handleStop: request
|
||||
// the AUTHORITATIVE server stop so the detached run settles, and arm the
|
||||
// one-shot bounded 409 retry BEFORE stop() so the re-send converges once
|
||||
// the slot frees. Read chatId live from chatIdRef (adopted at the `start`
|
||||
// chunk). If it is not known yet (brand-new chat, first moment of its
|
||||
// first turn), defer the server stop via stopPendingRef exactly as
|
||||
// handleStop does — the onServerChatId adoption effect fires it once the
|
||||
// id lands; the retry stays armed so the re-send still converges then.
|
||||
if (autonomousRunsEnabled) {
|
||||
supersedeRetryRef.current = true; // arm the bounded 409 retry
|
||||
if (chatIdRef.current) {
|
||||
onServerStop?.(chatIdRef.current);
|
||||
} else {
|
||||
// Same #234-F5 sub-window limitation documented in handleStop: if the
|
||||
// local abort below cancels the reader before the `start` chunk lands,
|
||||
// the adoption effect never runs and the deferred stop never fires. Not
|
||||
// a regression; at minimum we don't strand refs (the isStreaming effect
|
||||
// defuses stopPendingRef on the next turn start).
|
||||
stopPendingRef.current = true;
|
||||
}
|
||||
}
|
||||
stop(); // -> onFinish({ isAbort: true }) flushes the promoted head
|
||||
} else {
|
||||
// Nothing to interrupt: just send it now (no interrupt note).
|
||||
@@ -999,7 +884,7 @@ export default function ChatThread({
|
||||
sendMessageRef.current?.({ text: msg.text });
|
||||
}
|
||||
},
|
||||
[setQueue, stop, setResumedTurnPair, autonomousRunsEnabled, onServerStop],
|
||||
[setQueue, stop, setResumedTurnPair],
|
||||
);
|
||||
|
||||
// Stop the current turn. ALWAYS abort the local SSE (`stop()`) so the composer
|
||||
@@ -1059,13 +944,6 @@ export default function ChatThread({
|
||||
setStopNotice(null);
|
||||
flushOnAbortRef.current = false;
|
||||
interruptNextSendRef.current = false;
|
||||
// #396: symmetric with the other one-shot interrupt flags — defuse a stale
|
||||
// supersede arm that was set but whose expected re-POST never fired (the
|
||||
// turn finished in the same tick as the click, or the promoted head was
|
||||
// gone), so it can never leak into this (or a later) turn's send and retry
|
||||
// that send's genuine 409. A legit arm is consumed by the transport POST
|
||||
// branch before this new turn streams, so this does not clobber it.
|
||||
supersedeRetryRef.current = false;
|
||||
// #234 F5: a new turn is starting — drop any pending deferred-stop from a
|
||||
// previous turn that never adopted an id, so it can never fire against this
|
||||
// (or a later) unrelated turn's run. A deferred stop for the CURRENT turn is
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
insertTableRow,
|
||||
deleteTableRow,
|
||||
updateTableCell,
|
||||
findInvalidNode,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
import { searchInDoc, SearchOptions } from "./lib/page-search.js";
|
||||
import { withPageLock } from "./lib/page-lock.js";
|
||||
@@ -1660,6 +1661,27 @@ export class DocmostClient {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-write SHAPE gate (#409). Walk the WHOLE node tree with the shared
|
||||
* `findInvalidNode` and throw a rich, path-anchored error the instant a nested
|
||||
* node has an absent/unknown `type` (or an unknown mark) — the exact shape that
|
||||
* otherwise surfaces DEEP in the Yjs encode as the cryptic
|
||||
* `Unknown node type: undefined`, but only AFTER a collab session was opened
|
||||
* and a page lock taken. Calling this BEFORE `getCollabTokenWithReauth` /
|
||||
* `mutatePageContent` fails fast: no collab connection, no lock, deterministic
|
||||
* message. `op` names the tool for the message prefix (e.g. "patch_node").
|
||||
*
|
||||
* `findInvalidNode` derives its "known type" set from the very same
|
||||
* `docmostExtensions` the encode path uses, so a node this gate accepts is one
|
||||
* the encoder will accept too.
|
||||
*/
|
||||
private assertValidNodeShape(op: string, node: any): void {
|
||||
const bad = findInvalidNode(node);
|
||||
if (bad) {
|
||||
throw new Error(`${op}: invalid node — ${bad.summary}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace page content with a raw ProseMirror JSON document (lossless) and/or
|
||||
* update its title. Both `doc` and `title` are optional, but at least one must
|
||||
@@ -1711,6 +1733,12 @@ export class DocmostClient {
|
||||
// page on overwrite.
|
||||
this.validateDocStructure(doc);
|
||||
|
||||
// #409: beyond the string-`type` check above, reject a nested node whose
|
||||
// `type` is a string but NOT a known Docmost schema node (a typo/unknown
|
||||
// block) — the same `Unknown node type` the encoder throws — with a rich,
|
||||
// path-anchored message, still BEFORE any collab connection.
|
||||
this.assertValidNodeShape("update_page_json", doc);
|
||||
|
||||
// Sanitize URLs before writing. This closes the JSON-path bypass: unlike
|
||||
// the markdown link path (which TipTap sanitizes), raw JSON could otherwise
|
||||
// inject javascript:/data: link hrefs or media srcs straight into the doc.
|
||||
@@ -2131,6 +2159,14 @@ export class DocmostClient {
|
||||
target.attrs.id = nodeId;
|
||||
}
|
||||
|
||||
// #409: fail fast on a malformed node SHAPE (a nested child with an
|
||||
// absent/unknown `type`, e.g. a text leaf written as `{"text":"foo"}` with
|
||||
// no `"type":"text"`) BEFORE opening a collab session or taking the page
|
||||
// lock — the root-only `typeof node.type === "string"` check above never
|
||||
// sees nested children, and the encoder's `Unknown node type: undefined`
|
||||
// would otherwise only surface after the connection.
|
||||
this.assertValidNodeShape("patch_node", target);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the collab doc by the canonical UUID, never the slugId (#260).
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
@@ -2221,6 +2257,11 @@ export class DocmostClient {
|
||||
}
|
||||
}
|
||||
|
||||
// #409: fail fast on a malformed node SHAPE (a nested child with an
|
||||
// absent/unknown `type`) BEFORE opening a collab session or taking the page
|
||||
// lock — the root-only check above never sees nested children.
|
||||
this.assertValidNodeShape("insert_node", node);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the collab doc by the canonical UUID, never the slugId (#260).
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
|
||||
@@ -13,7 +13,11 @@ import { JSDOM } from "jsdom";
|
||||
import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
|
||||
import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
|
||||
import { withPageLock } from "./page-lock.js";
|
||||
import { sanitizeForYjs, findUnstorableAttr } from "@docmost/prosemirror-markdown";
|
||||
import {
|
||||
sanitizeForYjs,
|
||||
findUnstorableAttr,
|
||||
findInvalidNode,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
|
||||
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
|
||||
import { VerifyReport } from "./diff.js";
|
||||
@@ -28,11 +32,25 @@ export { markdownToProseMirror };
|
||||
* place. `label` names the stage that failed (diagnostic). `sanitizeForYjs`
|
||||
* already stripped `undefined` attrs, so a remaining failure is pinpointed via
|
||||
* `findUnstorableAttr`.
|
||||
*
|
||||
* Diagnostics precedence (#409): the dominant crash here is
|
||||
* `Unknown node type: undefined` — a nested node with an absent/unknown `type`
|
||||
* (a SHAPE problem, e.g. `{"text":"foo"}` missing `"type":"text"`). That points
|
||||
* at the node, not an attribute, so `findInvalidNode` is consulted FIRST and,
|
||||
* on a hit, yields a path-anchored node-shape message. Only when the document
|
||||
* shape is sound do we fall back to `findUnstorableAttr` (undefined/function/
|
||||
* symbol/bigint attr values); the generic "attribute likely holds a value Yjs
|
||||
* cannot store" sentence is the last resort.
|
||||
*/
|
||||
function unstorableYjsError(safe: any, label: string, e: unknown): Error {
|
||||
const base = `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.`;
|
||||
const badNode = findInvalidNode(safe);
|
||||
if (badNode) {
|
||||
return new Error(`${base} Invalid node: ${badNode.summary}`);
|
||||
}
|
||||
const bad = findUnstorableAttr(safe);
|
||||
return new Error(
|
||||
`Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
|
||||
`${base}${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -236,7 +236,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'heading {"type":"heading","attrs":{"level":2},"content":' +
|
||||
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
|
||||
'JSON object or a JSON string (both accepted). Cheaper and safer than ' +
|
||||
'JSON object or a JSON string (both accepted). EVERY node, including ' +
|
||||
'nested children, must carry a string `type` from the Docmost schema; ' +
|
||||
'text leaves are {"type":"text","text":"..."} (a bare {"text":"..."} is ' +
|
||||
'rejected up front). Cheaper and safer than ' +
|
||||
'replacing the whole document for one-block structural edits. Reversible: ' +
|
||||
'the previous version is kept in page history.',
|
||||
tier: 'deferred',
|
||||
@@ -282,7 +285,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' +
|
||||
'heading {"type":"heading","attrs":{"level":2},"content":' +
|
||||
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. EVERY node, ' +
|
||||
'including nested children, must carry a string `type` from the Docmost ' +
|
||||
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
|
||||
'{"text":"..."} is rejected up front). The node may be a ' +
|
||||
'JSON object or a JSON string (both accepted). Reversible via page history.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
@@ -705,7 +711,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' +
|
||||
'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' +
|
||||
'to update only the title (though prefer the rename-page tool for a title-only ' +
|
||||
'change). Supplying neither content nor title is an error. Reversible: ' +
|
||||
'change). Supplying neither content nor title is an error. EVERY node, ' +
|
||||
'including nested children, must carry a string `type` from the Docmost ' +
|
||||
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
|
||||
'{"text":"..."} is rejected up front). Reversible: ' +
|
||||
'the previous version is kept in page history.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
// Mock regression for the FAIL-FAST invalid-node validation (#409).
|
||||
//
|
||||
// A structural editor (patch_node / insert_node / update_page_json) given a doc
|
||||
// whose NESTED child has an absent/unknown `type` (the exact shape the Yjs
|
||||
// encoder rejects with `Unknown node type: undefined`) must throw a RICH,
|
||||
// path-anchored error BEFORE it ever opens a collab session or takes a page
|
||||
// lock. We prove the fail-fast by standing up a collab stack whose HTTP handler
|
||||
// records EVERY request: a correct fail-fast never even fetches the collab
|
||||
// token (which `getCollabTokenWithReauth`, called AFTER the validation, would
|
||||
// request), and never drives a document change on the Hocuspocus doc.
|
||||
//
|
||||
// The happy path (a well-formed doc) is exercised too: it must reach the collab
|
||||
// write and succeed, so the gate is not over-eager.
|
||||
//
|
||||
// findInvalidNode's per-shape summaries are unit-tested in the package
|
||||
// (test/find-invalid-node.test.ts); this exercises the END-TO-END wiring through
|
||||
// the real client methods.
|
||||
import { test, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import http from "node:http";
|
||||
import { WebSocketServer } from "ws";
|
||||
import { Hocuspocus } from "@hocuspocus/server";
|
||||
import { DocmostClient } from "../../build/client.js";
|
||||
import { buildYDoc } from "../../build/lib/collaboration.js";
|
||||
|
||||
// A minimal valid seed doc with a real block id, so the happy-path patch_node
|
||||
// finds its target.
|
||||
const SEED_ID = "seed-para-id";
|
||||
function seedDoc() {
|
||||
return {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
attrs: { id: SEED_ID },
|
||||
content: [{ type: "text", text: "seed" }],
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
// Stand up an HTTP server that authenticates + hands out a collab token AND
|
||||
// upgrades /collab to a Hocuspocus instance seeded with the doc. `state` records
|
||||
// whether the collab token was ever fetched (proving the write path was entered)
|
||||
// and whether the Hocuspocus doc ever changed.
|
||||
async function spawnCollabStack() {
|
||||
const state = { changed: false, collabTokenFetched: false };
|
||||
|
||||
const hocuspocus = new Hocuspocus({
|
||||
quiet: true,
|
||||
async onLoadDocument() {
|
||||
return buildYDoc(seedDoc());
|
||||
},
|
||||
async onChange() {
|
||||
state.changed = true;
|
||||
},
|
||||
});
|
||||
|
||||
const wss = new WebSocketServer({ noServer: true });
|
||||
|
||||
const server = http.createServer((req, res) => {
|
||||
let raw = "";
|
||||
req.on("data", (c) => (raw += c));
|
||||
req.on("end", () => {
|
||||
if (req.url === "/api/auth/login") {
|
||||
res.writeHead(200, {
|
||||
"Content-Type": "application/json",
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
});
|
||||
res.end(JSON.stringify({ success: true }));
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/auth/collab-token") {
|
||||
state.collabTokenFetched = true;
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ data: { token: "collab-jwt" } }));
|
||||
return;
|
||||
}
|
||||
res.writeHead(404, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ message: "not found" }));
|
||||
});
|
||||
});
|
||||
|
||||
server.on("upgrade", (request, socket, head) => {
|
||||
if (!request.url || !request.url.startsWith("/collab")) {
|
||||
socket.destroy();
|
||||
return;
|
||||
}
|
||||
wss.handleUpgrade(request, socket, head, (ws) => {
|
||||
hocuspocus.handleConnection(ws, request);
|
||||
});
|
||||
});
|
||||
|
||||
const baseURL = await new Promise((resolve) => {
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const { port } = server.address();
|
||||
resolve(`http://127.0.0.1:${port}/api`);
|
||||
});
|
||||
});
|
||||
|
||||
openStacks.push({ server, hocuspocus });
|
||||
return { state, baseURL };
|
||||
}
|
||||
|
||||
const openStacks = [];
|
||||
after(async () => {
|
||||
await Promise.all(
|
||||
openStacks.map(
|
||||
({ server, hocuspocus }) =>
|
||||
new Promise((resolve) => {
|
||||
server.close(() => {
|
||||
Promise.resolve(hocuspocus.destroy?.()).finally(resolve);
|
||||
});
|
||||
}),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
const PAGE = "11111111-1111-4111-8111-111111111111";
|
||||
|
||||
// A node whose NESTED text leaf is missing "type":"text" (dominant #409 shape).
|
||||
const nestedTypelessNode = () => ({
|
||||
type: "paragraph",
|
||||
content: [{ text: "oops", marks: [] }],
|
||||
});
|
||||
|
||||
// A node with a NESTED unknown type NAME (typo).
|
||||
const nestedUnknownTypeNode = () => ({
|
||||
type: "paragraph",
|
||||
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
|
||||
});
|
||||
|
||||
test("patch_node fails fast on a nested typeless node — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
await assert.rejects(
|
||||
() => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()),
|
||||
(err) => {
|
||||
assert.match(err.message, /patch_node: invalid node/);
|
||||
assert.match(err.message, /missing "type"/);
|
||||
assert.match(err.message, /content\[0\]/); // path-anchored
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
state.collabTokenFetched,
|
||||
false,
|
||||
"must NOT fetch a collab token — validation runs before getCollabTokenWithReauth",
|
||||
);
|
||||
assert.equal(state.changed, false, "the collab doc must never be written");
|
||||
});
|
||||
|
||||
test("insert_node fails fast on a nested UNKNOWN type — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
await assert.rejects(
|
||||
() =>
|
||||
client.insertNode(PAGE, nestedUnknownTypeNode(), {
|
||||
position: "append",
|
||||
}),
|
||||
(err) => {
|
||||
assert.match(err.message, /insert_node: invalid node/);
|
||||
assert.match(err.message, /unknown node type "paragraf"/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("update_page_json fails fast on a nested typeless node — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
const badDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", content: [{ text: "oops" }] }],
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => client.updatePageJson(PAGE, badDoc),
|
||||
(err) => {
|
||||
// update_page_json runs validateDocStructure first (string-type check),
|
||||
// which already rejects a typeless node — so the message may come from
|
||||
// either guard, but the write must not happen.
|
||||
assert.match(err.message, /type/i);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("update_page_json fails fast on a nested UNKNOWN type name — rich #409 message", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
// validateDocStructure passes (type is a string); assertValidNodeShape must
|
||||
// catch the unknown schema name and produce the rich path-anchored message.
|
||||
const badDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => client.updatePageJson(PAGE, badDoc),
|
||||
(err) => {
|
||||
assert.match(err.message, /update_page_json: invalid node/);
|
||||
assert.match(err.message, /unknown node type "paragraf"/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("patch_node with a well-formed node proceeds to the collab write", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
const result = await client.patchNode(PAGE, SEED_ID, {
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "replacement" }],
|
||||
});
|
||||
|
||||
assert.equal(result.success, true);
|
||||
assert.equal(result.replaced, 1);
|
||||
assert.equal(
|
||||
state.collabTokenFetched,
|
||||
true,
|
||||
"a valid node must reach the collab write path",
|
||||
);
|
||||
assert.equal(state.changed, true, "the collab doc must be written");
|
||||
});
|
||||
@@ -0,0 +1,82 @@
|
||||
// #409: unstorableYjsError diagnostics PRECEDENCE. The opaque Yjs encode failure
|
||||
// (`Unknown node type: undefined`) is a node-SHAPE problem, so the shared
|
||||
// findInvalidNode is consulted FIRST and yields a path-anchored node message;
|
||||
// only a shape-sound doc falls back to findUnstorableAttr (undefined/function/
|
||||
// etc. attr values). Exercised through `assertYjsEncodable`, which runs the same
|
||||
// encode + error-wrapping the live write path uses.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { assertYjsEncodable } from "../../build/lib/collaboration.js";
|
||||
import {
|
||||
findInvalidNode,
|
||||
findUnstorableAttr,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
|
||||
const doc = (...content) => ({ type: "doc", content });
|
||||
|
||||
test("a nested typeless node yields the rich node-shape message (not an attr hint)", () => {
|
||||
const bad = doc({
|
||||
type: "paragraph",
|
||||
content: [{ text: "oops", marks: [] }], // missing "type":"text"
|
||||
});
|
||||
assert.throws(
|
||||
() => assertYjsEncodable(bad),
|
||||
(err) => {
|
||||
assert.match(err.message, /Invalid node:/);
|
||||
assert.match(err.message, /missing "type"/);
|
||||
assert.match(err.message, /content\[0\]/);
|
||||
// It must NOT fall through to the generic attribute sentence.
|
||||
assert.doesNotMatch(err.message, /Offending attribute/);
|
||||
assert.doesNotMatch(
|
||||
err.message,
|
||||
/attribute likely holds a value Yjs cannot store/,
|
||||
);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("PRECEDENCE: a genuine undefined-attr case is a node-SHAPE-clean case, so the attr fallback fires", () => {
|
||||
// A doc whose node shapes are ALL valid but that carries a Yjs-unstorable
|
||||
// undefined attribute. unstorableYjsError checks findInvalidNode FIRST (must
|
||||
// miss here) and only then findUnstorableAttr (must hit) — this is exactly the
|
||||
// division of labor that keeps a real attr problem from being mislabelled as a
|
||||
// node-shape problem, and vice versa. We assert the two helpers directly (the
|
||||
// wrapper is not exported) because sanitizeForYjs strips undefined attrs before
|
||||
// the live encoder ever sees them, so this branch cannot be reached through
|
||||
// assertYjsEncodable without also failing the clone.
|
||||
const attrProblem = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1", indent: undefined },
|
||||
content: [{ type: "text", text: "hi" }],
|
||||
});
|
||||
// findInvalidNode: shape is clean -> null (so the wrapper does NOT emit
|
||||
// "Invalid node").
|
||||
assert.equal(findInvalidNode(attrProblem), null);
|
||||
// findUnstorableAttr: pinpoints the undefined attr -> the fallback message.
|
||||
assert.match(findUnstorableAttr(attrProblem) ?? "", /indent \(undefined\)/);
|
||||
});
|
||||
|
||||
test("PRECEDENCE: a node-shape problem is caught by findInvalidNode even when an attr is also unstorable", () => {
|
||||
// Both a shape problem (typeless nested leaf) AND an unstorable attr exist;
|
||||
// findInvalidNode wins, so the model is pointed at the node shape (the real
|
||||
// root cause of `Unknown node type: undefined`), not the attribute.
|
||||
const both = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1", indent: undefined },
|
||||
content: [{ text: "oops" }],
|
||||
});
|
||||
const shape = findInvalidNode(both);
|
||||
assert.notEqual(shape, null);
|
||||
assert.match(shape.summary, /missing "type"/);
|
||||
});
|
||||
|
||||
test("a fully valid document encodes without throwing", () => {
|
||||
const good = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1" },
|
||||
content: [{ type: "text", text: "hi" }],
|
||||
});
|
||||
assert.doesNotThrow(() => assertYjsEncodable(good));
|
||||
});
|
||||
@@ -56,6 +56,7 @@ export {
|
||||
deleteNodeById,
|
||||
sanitizeForYjs,
|
||||
findUnstorableAttr,
|
||||
findInvalidNode,
|
||||
insertNodeRelative,
|
||||
readTable,
|
||||
insertTableRow,
|
||||
|
||||
@@ -14,7 +14,9 @@
|
||||
* `content`, non-object nodes, and absent `attrs` are tolerated.
|
||||
*/
|
||||
|
||||
import { getSchema } from "@tiptap/core";
|
||||
import { stripInlineMarkdown } from "./text-normalize.js";
|
||||
import { docmostExtensions } from "./docmost-schema.js";
|
||||
|
||||
/** Deep-clone a JSON-serializable value without mutating the original. */
|
||||
function clone<T>(value: T): T {
|
||||
@@ -383,6 +385,119 @@ export function findUnstorableAttr(doc: any): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The Docmost schema's known node and mark NAME sets, derived ONCE from the very
|
||||
* same `docmostExtensions` the Yjs encode path builds its schema from
|
||||
* (`getSchema(docmostExtensions)` — mirrored in mcp's `docmostSchema`). Deriving
|
||||
* both from the same extension list guarantees `findInvalidNode`'s "known type"
|
||||
* set matches exactly what `PMNode.fromJSON`/`toYdoc` will actually accept, so
|
||||
* the walker never flags a node the encoder would have stored (or vice versa).
|
||||
* Lazy + cached: the schema is only built on first use.
|
||||
*/
|
||||
let schemaNames: { nodes: Set<string>; marks: Set<string> } | null = null;
|
||||
function getSchemaNames(): { nodes: Set<string>; marks: Set<string> } {
|
||||
if (schemaNames == null) {
|
||||
const schema = getSchema(docmostExtensions);
|
||||
schemaNames = {
|
||||
nodes: new Set(Object.keys(schema.nodes)),
|
||||
marks: new Set(Object.keys(schema.marks)),
|
||||
};
|
||||
}
|
||||
return schemaNames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Depth-first walk of the JSON `content` tree looking for the FIRST node whose
|
||||
* SHAPE the Yjs encode path will reject with an opaque
|
||||
* `Unknown node type: undefined` (issue #409). Returns `{ path, summary }` for
|
||||
* the offending node, or `null` when every node (and every mark) is a known
|
||||
* Docmost schema type.
|
||||
*
|
||||
* Two failure modes are detected, in order, per node:
|
||||
* 1. `type` is missing or not a string — the dominant `undefined` case, e.g.
|
||||
* a text leaf written as `{"text":"foo"}` with no `"type":"text"`.
|
||||
* 2. `type` is a string but NOT a known Docmost node name (a typo / unknown
|
||||
* block), OR one of the node's marks carries an unknown mark name.
|
||||
*
|
||||
* The returned `summary` is a model-actionable, path-anchored message such as:
|
||||
* `node.content[2].content[0]: missing "type" (keys: text, marks) — did you
|
||||
* mean {"type": "text", ...}?`
|
||||
* or for an unknown type:
|
||||
* `node.content[1]: unknown node type "paragraf" — not in the Docmost schema`
|
||||
*
|
||||
* `path` is the same dotted JSON path used in the summary (e.g.
|
||||
* `node.content[2].content[0]`) so callers can surface it separately. Null-safe:
|
||||
* a non-object doc returns `null`.
|
||||
*
|
||||
* NOTE: This is a SHAPE check, not a full ProseMirror content-model validation
|
||||
* (it does not verify that a paragraph may legally contain a table, etc.). Its
|
||||
* job is to turn the specific "unknown/absent node type" Yjs crash into a clear,
|
||||
* pre-write diagnostic; the schema's own `.check()` still catches deeper
|
||||
* content-model violations at encode time.
|
||||
*/
|
||||
export function findInvalidNode(
|
||||
doc: any,
|
||||
): { path: string; summary: string } | null {
|
||||
if (!isObject(doc)) return null;
|
||||
const { nodes, marks } = getSchemaNames();
|
||||
|
||||
// Build the "did you mean" hint for a typeless node from its own keys, so the
|
||||
// model sees WHICH object is malformed and the canonical text-leaf fix.
|
||||
const keyHint = (node: Record<string, any>): string => {
|
||||
const keys = Object.keys(node);
|
||||
const looksLikeText =
|
||||
typeof node.text === "string" && node.type === undefined;
|
||||
const suffix = looksLikeText
|
||||
? ` — did you mean {"type": "text", ...}?`
|
||||
: ` — every node needs a string "type" from the Docmost schema`;
|
||||
return `missing "type" (keys: ${keys.join(", ") || "none"})${suffix}`;
|
||||
};
|
||||
|
||||
const walk = (
|
||||
node: any,
|
||||
path: string,
|
||||
): { path: string; summary: string } | null => {
|
||||
if (!isObject(node)) return null;
|
||||
|
||||
// (1) missing / non-string type.
|
||||
if (typeof node.type !== "string") {
|
||||
return { path, summary: `${path}: ${keyHint(node)}` };
|
||||
}
|
||||
// (2) string type that is not a known Docmost node.
|
||||
if (!nodes.has(node.type)) {
|
||||
return {
|
||||
path,
|
||||
summary: `${path}: unknown node type "${node.type}" — not in the Docmost schema`,
|
||||
};
|
||||
}
|
||||
// (2b) unknown mark on an otherwise-valid node.
|
||||
if (Array.isArray(node.marks)) {
|
||||
for (let i = 0; i < node.marks.length; i++) {
|
||||
const mark = node.marks[i];
|
||||
if (isObject(mark) && typeof mark.type === "string" && !marks.has(mark.type)) {
|
||||
return {
|
||||
path: `${path}.marks[${i}]`,
|
||||
summary: `${path}.marks[${i}]: unknown mark type "${mark.type}" — not in the Docmost schema`,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (Array.isArray(node.content)) {
|
||||
for (let i = 0; i < node.content.length; i++) {
|
||||
const hit = walk(node.content[i], `${path}.content[${i}]`);
|
||||
if (hit != null) return hit;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
// The root doc node is addressed as "node" (matching the mcp arg name); its
|
||||
// children are node.content[i]. The root itself is checked too so a typeless
|
||||
// root is reported rather than silently skipped.
|
||||
return walk(doc, "node");
|
||||
}
|
||||
|
||||
/**
|
||||
* Table structural node types and the container each must live directly inside.
|
||||
* Used by `insertNodeRelative` to splice rows/cells into the correct ancestor
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { findInvalidNode } from '../src/lib/node-ops.js';
|
||||
|
||||
// findInvalidNode (#409): a depth-first SHAPE gate that turns the encoder's
|
||||
// opaque `Unknown node type: undefined` into a path-anchored, pre-write
|
||||
// diagnostic. It flags the FIRST node whose `type` is absent/non-string or not
|
||||
// a known Docmost schema node, or that carries an unknown mark; returns null for
|
||||
// a well-formed doc.
|
||||
|
||||
const doc = (...content: any[]) => ({ type: 'doc', content });
|
||||
const para = (...content: any[]) => ({
|
||||
type: 'paragraph',
|
||||
attrs: { id: 'p1' },
|
||||
content,
|
||||
});
|
||||
const text = (value: string, marks?: any[]) => {
|
||||
const node: any = { type: 'text', text: value };
|
||||
if (marks) node.marks = marks;
|
||||
return node;
|
||||
};
|
||||
|
||||
describe('findInvalidNode', () => {
|
||||
it('returns null for a fully valid document', () => {
|
||||
const good = doc(
|
||||
para(text('hello ', [{ type: 'bold' }]), text('world')),
|
||||
{
|
||||
type: 'heading',
|
||||
attrs: { id: 'h1', level: 2 },
|
||||
content: [text('Title')],
|
||||
},
|
||||
);
|
||||
expect(findInvalidNode(good)).toBeNull();
|
||||
});
|
||||
|
||||
it('flags a NESTED typeless text leaf with a path-precise summary', () => {
|
||||
// A text leaf written as {"text":"foo"} with no "type":"text" — the dominant
|
||||
// `Unknown node type: undefined` cause.
|
||||
const bad = doc(
|
||||
para(text('ok')),
|
||||
para({ text: 'foo', marks: [] } as any),
|
||||
);
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
// Second paragraph (index 1), first child (index 0).
|
||||
expect(hit!.path).toBe('node.content[1].content[0]');
|
||||
expect(hit!.summary).toContain('node.content[1].content[0]');
|
||||
expect(hit!.summary).toContain('missing "type"');
|
||||
expect(hit!.summary).toContain('keys: text, marks');
|
||||
expect(hit!.summary).toContain('did you mean {"type": "text", ...}');
|
||||
});
|
||||
|
||||
it('flags a non-string type (e.g. numeric)', () => {
|
||||
const bad = doc(para({ type: 123, content: [] } as any));
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0].content[0]');
|
||||
expect(hit!.summary).toContain('missing "type"');
|
||||
});
|
||||
|
||||
it('flags an UNKNOWN node type name that is not in the Docmost schema', () => {
|
||||
const bad = doc({
|
||||
type: 'paragraf', // typo — not a real node
|
||||
attrs: { id: 'x' },
|
||||
content: [text('hi')],
|
||||
});
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0]');
|
||||
expect(hit!.summary).toContain('unknown node type "paragraf"');
|
||||
expect(hit!.summary).toContain('not in the Docmost schema');
|
||||
});
|
||||
|
||||
it('flags an UNKNOWN mark type on an otherwise-valid node', () => {
|
||||
const bad = doc(para(text('hi', [{ type: 'blink' }])));
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0].content[0].marks[0]');
|
||||
expect(hit!.summary).toContain('unknown mark type "blink"');
|
||||
});
|
||||
|
||||
it('accepts every real Docmost node/mark type it is asked about', () => {
|
||||
// Known node (callout) and known marks (italic, code) must NOT be flagged.
|
||||
const good = doc({
|
||||
type: 'callout',
|
||||
attrs: { id: 'c1', type: 'info' },
|
||||
content: [para(text('x', [{ type: 'italic' }, { type: 'code' }]))],
|
||||
});
|
||||
expect(findInvalidNode(good)).toBeNull();
|
||||
});
|
||||
|
||||
it('reports the root itself when the root is typeless', () => {
|
||||
const hit = findInvalidNode({ content: [] } as any);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node');
|
||||
});
|
||||
|
||||
it('is null-safe for non-object input', () => {
|
||||
expect(findInvalidNode(null)).toBeNull();
|
||||
expect(findInvalidNode(undefined)).toBeNull();
|
||||
expect(findInvalidNode('nope' as any)).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user