Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 agent_coder
|
3cba551800 | ||
 vvzvlad
|
f8d37d8956 | ||
|
agent_coder
|
90168eb926 |
@@ -28,7 +28,10 @@ const h = vi.hoisted(() => ({
|
||||
body: Record<string, unknown>;
|
||||
}) => { body: Record<string, unknown> };
|
||||
prepareReconnectToStreamRequest?: () => { api?: string };
|
||||
fetch?: (input: unknown, init?: { method?: string }) => Promise<unknown>;
|
||||
fetch?: (
|
||||
input: unknown,
|
||||
init?: { method?: string; body?: unknown },
|
||||
) => Promise<unknown>;
|
||||
},
|
||||
},
|
||||
}));
|
||||
@@ -200,6 +203,244 @@ 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,6 +70,36 @@ 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;
|
||||
@@ -326,6 +356,26 @@ 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
|
||||
@@ -382,7 +432,43 @@ export default function ChatThread({
|
||||
}`,
|
||||
}),
|
||||
fetch: async (input: RequestInfo | URL, init: RequestInit = {}) => {
|
||||
if ((init.method ?? "GET") !== "GET") return fetch(input, init); // send path untouched
|
||||
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]),
|
||||
);
|
||||
}
|
||||
}
|
||||
// Reconnect GET: the SDK passes no AbortSignal, so wire our own controller
|
||||
// for observer Stop / unmount abort.
|
||||
const controller = new AbortController();
|
||||
@@ -562,9 +648,14 @@ 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 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;
|
||||
// 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;
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (isAbort || isDisconnect || isError) return;
|
||||
@@ -873,6 +964,30 @@ 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).
|
||||
@@ -884,7 +999,7 @@ export default function ChatThread({
|
||||
sendMessageRef.current?.({ text: msg.text });
|
||||
}
|
||||
},
|
||||
[setQueue, stop, setResumedTurnPair],
|
||||
[setQueue, stop, setResumedTurnPair, autonomousRunsEnabled, onServerStop],
|
||||
);
|
||||
|
||||
// Stop the current turn. ALWAYS abort the local SSE (`stop()`) so the composer
|
||||
@@ -944,6 +1059,13 @@ 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
|
||||
|
||||
+169
-78
@@ -553,16 +553,18 @@ export class DocmostClient {
|
||||
// forever and accumulate duplicates).
|
||||
const MAX_PAGES = 50;
|
||||
|
||||
let page = 1;
|
||||
let cursor: string | undefined;
|
||||
let allItems: T[] = [];
|
||||
let hasNextPage = true;
|
||||
let truncated = false;
|
||||
|
||||
while (hasNextPage && page <= MAX_PAGES) {
|
||||
const response = await this.client.post(endpoint, {
|
||||
for (let page = 0; page < MAX_PAGES; page++) {
|
||||
const payload: Record<string, any> = {
|
||||
...basePayload,
|
||||
limit: clampedLimit,
|
||||
page,
|
||||
});
|
||||
};
|
||||
if (cursor) payload.cursor = cursor;
|
||||
|
||||
const response = await this.client.post(endpoint, payload);
|
||||
|
||||
const data = response.data;
|
||||
const items = data.data?.items || data.items || [];
|
||||
@@ -570,22 +572,28 @@ export class DocmostClient {
|
||||
|
||||
allItems = allItems.concat(items);
|
||||
|
||||
// Stop if the page is empty or shorter than the requested size: a full
|
||||
// page worth of items is the only situation where another page can exist,
|
||||
// so this defends against a stuck hasNextPage flag in addition to it.
|
||||
if (items.length === 0 || items.length < clampedLimit) {
|
||||
// Advance strictly via the server-issued cursor. A missing nextCursor (or
|
||||
// hasNextPage false) means we reached the end. A cursor identical to the
|
||||
// one we just sent means the server did not understand our pagination
|
||||
// param — stop instead of re-fetching page one forever and duplicating.
|
||||
const next = meta?.hasNextPage ? meta?.nextCursor : null;
|
||||
if (!next || next === cursor) {
|
||||
// If the server still reports more pages but stopped issuing a usable
|
||||
// cursor at the ceiling, flag the result as truncated below.
|
||||
if (page === MAX_PAGES - 1 && meta?.hasNextPage) truncated = true;
|
||||
break;
|
||||
}
|
||||
cursor = next;
|
||||
|
||||
hasNextPage = meta?.hasNextPage || false;
|
||||
page++;
|
||||
// Reaching the ceiling with more pages still available means the result
|
||||
// set is truncated.
|
||||
if (page === MAX_PAGES - 1) truncated = true;
|
||||
}
|
||||
|
||||
// If the loop stopped because it hit the MAX_PAGES ceiling while the server
|
||||
// still reported more results (hasNextPage true and the last page was
|
||||
// full), the result set is truncated — warn so the caller is not silently
|
||||
// handed an incomplete list.
|
||||
if (hasNextPage && page > MAX_PAGES) {
|
||||
// still reported more results, the result set is truncated — warn so the
|
||||
// caller is not silently handed an incomplete list.
|
||||
if (truncated) {
|
||||
console.warn(
|
||||
`paginateAll: results from "${endpoint}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
|
||||
);
|
||||
@@ -619,9 +627,10 @@ export class DocmostClient {
|
||||
* Tree (`tree` true): the space's FULL page hierarchy as a nested tree (each
|
||||
* node has a `children` array). This mode REQUIRES `spaceId` (a page tree is
|
||||
* scoped to one space) and IGNORES `limit` — the whole hierarchy is returned.
|
||||
* It walks the sidebar tree via `enumerateSpacePages`, which performs N
|
||||
* sidebar requests and is bounded by that method's 10000-node cap (and skips
|
||||
* soft-deleted pages server-side).
|
||||
* It fetches the tree via `enumerateSpacePages`, which on the fork server
|
||||
* resolves to a single `/pages/tree` request returning the whole
|
||||
* permission-filtered flat page set (soft-deleted pages excluded
|
||||
* server-side).
|
||||
*/
|
||||
async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
|
||||
await this.ensureAuthenticated();
|
||||
@@ -632,8 +641,8 @@ export class DocmostClient {
|
||||
"list_pages: tree mode requires a spaceId (a page tree is scoped to one space). Pass spaceId, or omit tree to get the recent-pages list.",
|
||||
);
|
||||
}
|
||||
const nodes = await this.enumerateSpacePages(spaceId);
|
||||
return buildPageTree(nodes);
|
||||
const { pages } = await this.enumerateSpacePages(spaceId);
|
||||
return buildPageTree(pages);
|
||||
}
|
||||
|
||||
const clampedLimit = Math.max(1, Math.min(100, limit));
|
||||
@@ -655,57 +664,123 @@ export class DocmostClient {
|
||||
async listSidebarPages(spaceId: string, pageId?: string) {
|
||||
await this.ensureAuthenticated();
|
||||
|
||||
// Paginate: the endpoint returns server-paged children, so posting only
|
||||
// { page: 1 } silently dropped every child beyond the first page. Loop on
|
||||
// meta.hasNextPage (with a MAX_PAGES ceiling like paginateAll, guarding
|
||||
// against a stuck hasNextPage flag) and accumulate all children.
|
||||
// Paginate via the server-issued cursor. The server switched from OFFSET
|
||||
// (`page`) to CURSOR (`cursor`/`nextCursor`) pagination, and the global
|
||||
// ValidationPipe(whitelist:true) SILENTLY STRIPS the obsolete `page` field
|
||||
// — so the old offset loop got the SAME first page every time (with
|
||||
// hasNextPage stuck true) and dropped every child beyond the first page.
|
||||
const MAX_PAGES = 50;
|
||||
let page = 1;
|
||||
let cursor: string | undefined;
|
||||
let allItems: any[] = [];
|
||||
let hasNextPage = true;
|
||||
let truncated = false;
|
||||
|
||||
while (hasNextPage && page <= MAX_PAGES) {
|
||||
for (let i = 0; i < MAX_PAGES; i++) {
|
||||
// limit: 100 is the server-side Max; cuts request count 5x vs the default 20.
|
||||
const payload: Record<string, any> = { spaceId, limit: 100 };
|
||||
// Only send pageId when scoping to a page's children; omit it for roots.
|
||||
const payload: Record<string, any> = { spaceId, page };
|
||||
if (pageId) payload.pageId = pageId;
|
||||
if (cursor) payload.cursor = cursor;
|
||||
|
||||
const response = await this.client.post("/pages/sidebar-pages", payload);
|
||||
const data = response.data?.data ?? response.data;
|
||||
const items = data?.items || [];
|
||||
allItems = allItems.concat(items);
|
||||
const data = (await this.client.post("/pages/sidebar-pages", payload)).data
|
||||
?.data;
|
||||
allItems = allItems.concat(data?.items ?? []);
|
||||
|
||||
hasNextPage = data?.meta?.hasNextPage || false;
|
||||
page++;
|
||||
// Advance strictly via the server-issued cursor; a missing/repeated cursor
|
||||
// means the protocol drifted again — stop instead of looping on page one.
|
||||
const next = data?.meta?.hasNextPage ? data?.meta?.nextCursor : null;
|
||||
if (!next || next === cursor) break;
|
||||
cursor = next;
|
||||
|
||||
// Reaching the ceiling with more pages still available means the child
|
||||
// list is truncated (mirrors paginateAll).
|
||||
if (i === MAX_PAGES - 1) truncated = true;
|
||||
}
|
||||
|
||||
// Warn on real truncation (ceiling hit while the server still had pages) so
|
||||
// the caller is not silently handed an incomplete child list.
|
||||
if (truncated) {
|
||||
console.warn(
|
||||
`listSidebarPages: children of "${pageId ?? spaceId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
|
||||
);
|
||||
}
|
||||
|
||||
return allItems;
|
||||
}
|
||||
|
||||
/**
|
||||
* Enumerate EVERY page in a space (or in a subtree, when rootPageId is given)
|
||||
* by walking the sidebar-pages tree.
|
||||
* Enumerate EVERY page in a space (or in a subtree, when rootPageId is given).
|
||||
*
|
||||
* Starting set: the children of rootPageId when provided, otherwise the
|
||||
* space root pages. From there it does an iterative breadth-first walk: each
|
||||
* node is collected, and when node.hasChildren is true its direct children
|
||||
* are fetched via listSidebarPages(spaceId, node.id) and enqueued.
|
||||
* Primary path (fork server): a SINGLE `POST /pages/tree` returns the whole
|
||||
* space (or a subtree) as a flat, permission-filtered list in one request, in
|
||||
* the exact node shape buildPageTree consumes. This replaces the old
|
||||
* per-node BFS, which issued N sidebar requests and — after the server moved
|
||||
* to cursor pagination — silently lost every child past the first sidebar
|
||||
* page (the obsolete `page` param was stripped by ValidationPipe).
|
||||
*
|
||||
* This replaces the old "/pages/recent" enumeration, which is a bounded
|
||||
* recent-activity feed (~5000 cap) and therefore misses comments on older
|
||||
* pages that were never recently touched.
|
||||
* The subtree variant (rootPageId given) INCLUDES the root node itself
|
||||
* (getPageAndDescendants seeds with id = rootPageId), unlike the old BFS
|
||||
* which started from the root's children.
|
||||
*
|
||||
* Safeguards: a `visited` Set of page ids prevents re-processing a node
|
||||
* (cycles / duplicate references), and a hard node cap bounds pathological
|
||||
* trees so the walk always terminates.
|
||||
* Fallback path (stdio mode may target STOCK upstream Docmost, which lacks
|
||||
* `/pages/tree`): on a 404/405 it falls back to the cursor-based BFS below,
|
||||
* walking direct children via the fixed cursor listSidebarPages. Safeguards:
|
||||
* a `visited` Set of page ids prevents re-processing a node (cycles /
|
||||
* duplicate references), and a hard node cap bounds pathological trees so the
|
||||
* walk always terminates.
|
||||
*
|
||||
* Returns `{ pages, truncated }`. `truncated` is true ONLY when the fallback
|
||||
* BFS stopped at its MAX_NODES cap — the primary /pages/tree path is uncapped
|
||||
* and always returns the complete set, so it never reports truncation.
|
||||
*/
|
||||
private async enumerateSpacePages(
|
||||
spaceId: string,
|
||||
rootPageId?: string,
|
||||
): Promise<any[]> {
|
||||
): Promise<{ pages: any[]; truncated: boolean }> {
|
||||
await this.ensureAuthenticated();
|
||||
|
||||
// Single request replaces the whole BFS: /pages/tree returns the full
|
||||
// permission-filtered flat page set of a space (or a subtree) at once. This
|
||||
// path is uncapped, so it is never truncated.
|
||||
const payload = rootPageId ? { pageId: rootPageId } : { spaceId };
|
||||
try {
|
||||
const response = await this.client.post("/pages/tree", payload);
|
||||
const pages = (response.data?.data ?? response.data)?.items ?? [];
|
||||
return { pages, truncated: false };
|
||||
} catch (e: any) {
|
||||
// Only fall back when the endpoint is absent (stock upstream Docmost);
|
||||
// any other error is a genuine failure and must propagate.
|
||||
if (
|
||||
!axios.isAxiosError(e) ||
|
||||
(e.response?.status !== 404 && e.response?.status !== 405)
|
||||
) {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback: cursor-based breadth-first walk via listSidebarPages.
|
||||
const MAX_NODES = 10000;
|
||||
const result: any[] = [];
|
||||
const visited = new Set<string>();
|
||||
|
||||
// Seed with the root node itself when scoping to a subtree, so its own
|
||||
// comments aren't dropped: the primary /pages/tree seeds
|
||||
// getPageAndDescendants with id = rootPageId (root included), but
|
||||
// listSidebarPages(spaceId, rootPageId) returns only the root's CHILDREN.
|
||||
// The `visited` set below prevents a double-add if the root also appears
|
||||
// among the children. getPageRaw returns a page whose id/title/spaceId are
|
||||
// exactly what buildPageTree and check_new_comments consume.
|
||||
if (rootPageId) {
|
||||
try {
|
||||
const root = await this.getPageRaw(rootPageId);
|
||||
if (root?.id) {
|
||||
result.push(root);
|
||||
visited.add(root.id);
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal: if the root can't be read, fall through to children-only.
|
||||
}
|
||||
}
|
||||
|
||||
// Seed the queue with the starting level (subtree children or roots).
|
||||
const queue: any[] = await this.listSidebarPages(spaceId, rootPageId);
|
||||
|
||||
@@ -730,7 +805,12 @@ export class DocmostClient {
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
// Truncated only when the cap was hit with the queue still non-empty (real
|
||||
// truncation, not a natural end at exactly MAX_NODES).
|
||||
return {
|
||||
pages: result,
|
||||
truncated: result.length >= MAX_NODES && queue.length > 0,
|
||||
};
|
||||
}
|
||||
|
||||
/** Raw page info including the ProseMirror JSON content and slugId. */
|
||||
@@ -2364,7 +2444,13 @@ export class DocmostClient {
|
||||
let allComments: any[] = [];
|
||||
let cursor: string | null = null;
|
||||
|
||||
do {
|
||||
// Hard ceiling + immovable-cursor guard (mirrors paginateAll): if /comments
|
||||
// ever stops advancing the cursor (the exact #442 drift scenario) this loop
|
||||
// would otherwise spin forever accumulating duplicates.
|
||||
const MAX_PAGES = 50;
|
||||
let truncated = false;
|
||||
|
||||
for (let page = 0; page < MAX_PAGES; page++) {
|
||||
const payload: Record<string, any> = { pageId, limit: 100 };
|
||||
if (cursor) payload.cursor = cursor;
|
||||
|
||||
@@ -2372,8 +2458,23 @@ export class DocmostClient {
|
||||
const data = response.data.data || response.data;
|
||||
const items = data.items || [];
|
||||
allComments = allComments.concat(items);
|
||||
cursor = data.meta?.nextCursor || null;
|
||||
} while (cursor);
|
||||
|
||||
// Advance strictly via the server-issued cursor. A missing nextCursor or a
|
||||
// cursor identical to the one we just sent means the end (or a server that
|
||||
// ignores our pagination param) — stop instead of re-fetching page one.
|
||||
const next: string | null = data.meta?.nextCursor || null;
|
||||
if (!next || next === cursor) break;
|
||||
cursor = next;
|
||||
|
||||
// Reaching the ceiling with a still-advancing cursor means truncation.
|
||||
if (page === MAX_PAGES - 1) truncated = true;
|
||||
}
|
||||
|
||||
if (truncated) {
|
||||
console.warn(
|
||||
`listComments: comments for "${pageId}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`,
|
||||
);
|
||||
}
|
||||
|
||||
const mapped = allComments.map((comment: any) => {
|
||||
const markdown = comment.content
|
||||
@@ -2846,36 +2947,27 @@ export class DocmostClient {
|
||||
);
|
||||
}
|
||||
|
||||
// 1. Enumerate the FULL set of pages in scope by walking the sidebar-pages
|
||||
// tree (a complete page index), NOT the bounded "/pages/recent" feed which
|
||||
// caps at ~5000 recent items and silently misses comments on older pages.
|
||||
// 1. Enumerate the FULL set of pages in scope via the page tree (a complete
|
||||
// page index), NOT the bounded "/pages/recent" feed which caps at ~5000
|
||||
// recent items and silently misses comments on older pages.
|
||||
//
|
||||
// Subtree scope: when parentPageId is given, the scope is that page ITSELF
|
||||
// plus every descendant (enumerateSpacePages walks its children). Otherwise
|
||||
// the scope is the whole space (all roots and their descendants).
|
||||
// plus every descendant. Otherwise the scope is the whole space (all roots
|
||||
// and their descendants).
|
||||
//
|
||||
// NOTE: do NOT pre-filter by page.updatedAt — creating a comment does not
|
||||
// bump it (verified on a live server), so such a filter silently misses
|
||||
// comments on pages that were not otherwise edited. The complete tree walk
|
||||
// already restricts the scope correctly, so no recent-feed allow-list is
|
||||
// needed any more.
|
||||
let pagesInScope: any[];
|
||||
if (parentPageId) {
|
||||
const subtree = await this.enumerateSpacePages(spaceId, parentPageId);
|
||||
// Include the parent page node itself alongside its descendants. Fetch it
|
||||
// so its title/id are available even though it is not returned by its own
|
||||
// children listing.
|
||||
let parentNode: any = { id: parentPageId };
|
||||
try {
|
||||
parentNode = await this.getPageRaw(parentPageId);
|
||||
} catch (e: any) {
|
||||
// Fall back to a minimal node if the parent can't be fetched; its
|
||||
// comments are still attempted below (the fetch there is non-fatal).
|
||||
}
|
||||
pagesInScope = [parentNode, ...subtree];
|
||||
} else {
|
||||
pagesInScope = await this.enumerateSpacePages(spaceId);
|
||||
}
|
||||
//
|
||||
// The subtree scope (parentPageId given) already INCLUDES the root node
|
||||
// itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
|
||||
// no separate getPageRaw fetch for the parent is needed.
|
||||
const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
|
||||
spaceId,
|
||||
parentPageId,
|
||||
);
|
||||
|
||||
// 2. Fetch comments for each page, keep ones created after since
|
||||
const results: any[] = [];
|
||||
@@ -2904,10 +2996,9 @@ export class DocmostClient {
|
||||
0,
|
||||
);
|
||||
|
||||
// enumerateSpacePages caps traversal at 10000 nodes; flag when that cap was
|
||||
// hit so the caller knows the scan may be incomplete (some pages skipped).
|
||||
const truncated = pagesInScope.length >= 10000;
|
||||
|
||||
// `truncated` is reported by enumerateSpacePages: it is true ONLY when the
|
||||
// stdio fallback BFS hit its node cap. The primary /pages/tree path is
|
||||
// uncapped, so a space with legitimately many pages is not falsely flagged.
|
||||
return {
|
||||
since,
|
||||
scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`,
|
||||
|
||||
@@ -0,0 +1,440 @@
|
||||
// Mock-HTTP tests for the cursor-pagination migration in DocmostClient (#442).
|
||||
//
|
||||
// The server switched its list endpoints from OFFSET (`page`) to CURSOR
|
||||
// (`cursor`/`nextCursor`) pagination, and the global ValidationPipe silently
|
||||
// strips the obsolete `page` field — so the old offset loops re-fetched page
|
||||
// one forever (hasNextPage stuck true), dropping every item past the first
|
||||
// page. These tests pin the new cursor behaviour and the immovable-cursor
|
||||
// guard that prevents a silent spin/duplication if the protocol drifts again.
|
||||
//
|
||||
// A local http.createServer stands in for Docmost so everything stays
|
||||
// deterministic and offline (same harness style as reauth.test.mjs).
|
||||
import { test, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import http from "node:http";
|
||||
import { DocmostClient } from "../../build/client.js";
|
||||
|
||||
function readBody(req) {
|
||||
return new Promise((resolve) => {
|
||||
let raw = "";
|
||||
req.on("data", (chunk) => {
|
||||
raw += chunk;
|
||||
});
|
||||
req.on("end", () => resolve(raw));
|
||||
});
|
||||
}
|
||||
|
||||
function startServer(handler) {
|
||||
return new Promise((resolve) => {
|
||||
const server = http.createServer(handler);
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const { port } = server.address();
|
||||
resolve({ server, baseURL: `http://127.0.0.1:${port}/api` });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function closeServer(server) {
|
||||
return new Promise((resolve) => server.close(resolve));
|
||||
}
|
||||
|
||||
function sendJson(res, status, obj, extraHeaders = {}) {
|
||||
res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders });
|
||||
res.end(JSON.stringify(obj));
|
||||
}
|
||||
|
||||
const openServers = [];
|
||||
async function spawn(handler) {
|
||||
const { server, baseURL } = await startServer(handler);
|
||||
openServers.push(server);
|
||||
return { server, baseURL };
|
||||
}
|
||||
|
||||
after(async () => {
|
||||
await Promise.all(openServers.map((s) => closeServer(s)));
|
||||
});
|
||||
|
||||
// A login handler shared by every server below.
|
||||
function handleLogin(req, res) {
|
||||
if (req.url === "/api/auth/login") {
|
||||
sendJson(res, 200, { success: true }, {
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
});
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 1) listSidebarPages: collects every cursor page; #requests == #pages.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("listSidebarPages walks all cursor pages and collects every item", async () => {
|
||||
// Three pages keyed by the cursor the client sends back.
|
||||
const PAGES = {
|
||||
"": { items: [{ id: "a" }, { id: "b" }], nextCursor: "c1" },
|
||||
c1: { items: [{ id: "c" }, { id: "d" }], nextCursor: "c2" },
|
||||
c2: { items: [{ id: "e" }], nextCursor: null },
|
||||
};
|
||||
let requests = 0;
|
||||
const sentLimits = [];
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
const raw = await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/sidebar-pages") {
|
||||
requests++;
|
||||
const body = JSON.parse(raw || "{}");
|
||||
sentLimits.push(body.limit);
|
||||
const page = PAGES[body.cursor ?? ""] ?? { items: [], nextCursor: null };
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: {
|
||||
items: page.items,
|
||||
meta: {
|
||||
hasNextPage: page.nextCursor != null,
|
||||
nextCursor: page.nextCursor,
|
||||
},
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const all = await client.listSidebarPages("space-1");
|
||||
|
||||
assert.equal(requests, 3, "one request per cursor page");
|
||||
assert.deepEqual(
|
||||
all.map((p) => p.id),
|
||||
["a", "b", "c", "d", "e"],
|
||||
"all items across all pages collected in order",
|
||||
);
|
||||
assert.ok(
|
||||
sentLimits.every((l) => l === 100),
|
||||
"requests limit:100 (server-side max)",
|
||||
);
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 2) REGRESSION on the bug class: server IGNORES the cursor param and always
|
||||
// returns page one with hasNextPage:true -> the immovable-cursor guard must
|
||||
// terminate the loop with no duplicates, NOT spin to MAX_PAGES.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("listSidebarPages terminates (no dups) when the server ignores the cursor", async () => {
|
||||
let requests = 0;
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/sidebar-pages") {
|
||||
requests++;
|
||||
// Always the SAME first page with hasNextPage:true and the SAME cursor,
|
||||
// exactly as a server that no longer understands our pagination param.
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: {
|
||||
items: [{ id: "x1" }, { id: "x2" }],
|
||||
meta: { hasNextPage: true, nextCursor: "stuck" },
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const all = await client.listSidebarPages("space-1");
|
||||
|
||||
// Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
|
||||
// again -> guard trips. Far below the MAX_PAGES=50 ceiling; no runaway dups.
|
||||
assert.equal(requests, 2, "stops as soon as the cursor stops moving");
|
||||
assert.equal(all.length, 4, "no runaway accumulation / duplication");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 3a) enumerateSpacePages happy path: a SINGLE /pages/tree request.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("enumerateSpacePages (via list_pages tree) uses one /pages/tree request", async () => {
|
||||
let treeRequests = 0;
|
||||
let sidebarRequests = 0;
|
||||
let treeBody = null;
|
||||
|
||||
const NODES = [
|
||||
{ id: "r1", slugId: "r1s", title: "Root 1", parentPageId: null, hasChildren: true, spaceId: "space-1", position: "a", icon: null, canEdit: true },
|
||||
{ id: "c1", slugId: "c1s", title: "Child 1", parentPageId: "r1", hasChildren: false, spaceId: "space-1", position: "a", icon: null, canEdit: true },
|
||||
{ id: "r2", slugId: "r2s", title: "Root 2", parentPageId: null, hasChildren: false, spaceId: "space-1", position: "b", icon: null, canEdit: true },
|
||||
];
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
const raw = await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/tree") {
|
||||
treeRequests++;
|
||||
treeBody = JSON.parse(raw || "{}");
|
||||
sendJson(res, 200, { success: true, data: { items: NODES } });
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/sidebar-pages") {
|
||||
sidebarRequests++;
|
||||
sendJson(res, 200, { success: true, data: { items: [], meta: {} } });
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
// list_pages tree:true -> enumerateSpacePages(spaceId) -> buildPageTree.
|
||||
const tree = await client.listPages("space-1", 50, true);
|
||||
|
||||
assert.equal(treeRequests, 1, "exactly one /pages/tree request for the space");
|
||||
assert.equal(sidebarRequests, 0, "no per-node sidebar BFS requests");
|
||||
assert.deepEqual(treeBody, { spaceId: "space-1" }, "space scope posts spaceId only");
|
||||
// buildPageTree nests c1 under r1; two roots at the top level.
|
||||
assert.equal(tree.length, 2, "two root nodes");
|
||||
const r1 = tree.find((n) => n.id === "r1");
|
||||
assert.equal(r1.children.length, 1, "child nested under its root");
|
||||
assert.equal(r1.children[0].id, "c1");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 3b) enumerateSpacePages fallback: /pages/tree 404 -> cursor BFS via sidebar.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("enumerateSpacePages falls back to the cursor BFS on /pages/tree 404", async () => {
|
||||
let treeRequests = 0;
|
||||
const sidebarCalls = [];
|
||||
|
||||
// Root level: one root with children. Child level (pageId=r1): one leaf.
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
const raw = await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/tree") {
|
||||
treeRequests++;
|
||||
// Stock upstream Docmost has no /pages/tree.
|
||||
sendJson(res, 404, { message: "Not Found" });
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/sidebar-pages") {
|
||||
const body = JSON.parse(raw || "{}");
|
||||
sidebarCalls.push(body.pageId ?? "<root>");
|
||||
if (!body.pageId) {
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: {
|
||||
items: [
|
||||
{ id: "r1", title: "Root", parentPageId: null, hasChildren: true },
|
||||
],
|
||||
meta: { hasNextPage: false, nextCursor: null },
|
||||
},
|
||||
});
|
||||
} else if (body.pageId === "r1") {
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: {
|
||||
items: [
|
||||
{ id: "c1", title: "Leaf", parentPageId: "r1", hasChildren: false },
|
||||
],
|
||||
meta: { hasNextPage: false, nextCursor: null },
|
||||
},
|
||||
});
|
||||
} else {
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items: [], meta: { hasNextPage: false, nextCursor: null } },
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const tree = await client.listPages("space-1", 50, true);
|
||||
|
||||
assert.ok(treeRequests >= 1, "the tree endpoint was attempted first");
|
||||
assert.deepEqual(
|
||||
sidebarCalls,
|
||||
["<root>", "r1"],
|
||||
"fell back to the sidebar BFS: roots then the root's children",
|
||||
);
|
||||
assert.equal(tree.length, 1, "one root in the built tree");
|
||||
assert.equal(tree[0].children[0].id, "c1", "leaf nested via the BFS");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 3c) enumerateSpacePages fallback SUBTREE: /pages/tree 404 + a rootPageId ->
|
||||
// the ROOT page itself must be seeded (via getPageRaw) so its own comments
|
||||
// aren't dropped. listSidebarPages(spaceId, root) returns only the root's
|
||||
// CHILDREN, so without the seed the root would be absent. (Finding 1.)
|
||||
// -----------------------------------------------------------------------------
|
||||
test("enumerateSpacePages fallback subtree seeds the ROOT page itself", async () => {
|
||||
const sidebarCalls = [];
|
||||
let infoRequests = 0;
|
||||
const commentedPages = [];
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
const raw = await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/tree") {
|
||||
// Stock upstream Docmost -> fall back to the BFS.
|
||||
sendJson(res, 404, { message: "Not Found" });
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/info") {
|
||||
// getPageRaw for the root seed. Shape mirrors a real page-info response.
|
||||
infoRequests++;
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { id: "root", title: "Root", spaceId: "space-1", hasChildren: true },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/sidebar-pages") {
|
||||
const body = JSON.parse(raw || "{}");
|
||||
sidebarCalls.push(body.pageId ?? "<root>");
|
||||
// Children of the root: one leaf. (Root itself is NOT in this list.)
|
||||
const items =
|
||||
body.pageId === "root"
|
||||
? [{ id: "leaf", title: "Leaf", parentPageId: "root", hasChildren: false }]
|
||||
: [];
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items, meta: { hasNextPage: false, nextCursor: null } },
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/comments") {
|
||||
const body = JSON.parse(raw || "{}");
|
||||
commentedPages.push(body.pageId);
|
||||
const items =
|
||||
body.pageId === "root"
|
||||
? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
|
||||
: [];
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items, meta: { nextCursor: null } },
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
// checkNewComments(space, since, parentPageId) exercises the subtree fallback.
|
||||
const result = await client.checkNewComments(
|
||||
"space-1",
|
||||
"2020-01-01T00:00:00.000Z",
|
||||
"root",
|
||||
);
|
||||
|
||||
assert.equal(infoRequests, 1, "root was seeded via one getPageRaw");
|
||||
assert.equal(sidebarCalls[0], "root", "BFS walked the root's children");
|
||||
assert.ok(
|
||||
commentedPages.includes("root"),
|
||||
"the ROOT page is in scope (its comments were fetched) — not dropped",
|
||||
);
|
||||
assert.ok(commentedPages.includes("leaf"), "the descendant is in scope too");
|
||||
assert.equal(result.checkedPages, 2, "root + one descendant scanned");
|
||||
assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 5) listComments immovable-cursor guard: the server IGNORES the cursor and
|
||||
// keeps returning the same nextCursor -> the loop must terminate (no
|
||||
// infinite loop, no duplicates), not spin forever. (Finding 4.)
|
||||
// -----------------------------------------------------------------------------
|
||||
test("listComments terminates (no dups) when the server ignores the cursor", async () => {
|
||||
let requests = 0;
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/comments") {
|
||||
requests++;
|
||||
// Always the SAME page with the SAME nextCursor, as a server that no
|
||||
// longer advances the cursor would.
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: {
|
||||
items: [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }],
|
||||
meta: { nextCursor: "stuck" },
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const { items } = await client.listComments("page-1", true);
|
||||
|
||||
// Request 1 (no cursor) gets "stuck"; request 2 (cursor "stuck") gets "stuck"
|
||||
// again -> guard trips. Bounded far below MAX_PAGES=50, no runaway dups.
|
||||
assert.equal(requests, 2, "stops as soon as the cursor stops moving");
|
||||
assert.equal(items.length, 2, "no runaway accumulation / duplication");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 4) check_new_comments subtree: the root is included in scope WITHOUT a
|
||||
// separate getPageRaw (/pages/info) request for the parent.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("checkNewComments subtree includes the root without a separate getPageRaw", async () => {
|
||||
let pageInfoRequests = 0;
|
||||
let treeBody = null;
|
||||
const commentedPages = [];
|
||||
|
||||
// /pages/tree (subtree) returns the parent itself plus a descendant, exactly
|
||||
// as getPageAndDescendants seeds with id = parentPageId.
|
||||
const NODES = [
|
||||
{ id: "parent", title: "Parent", parentPageId: null, hasChildren: true },
|
||||
{ id: "kid", title: "Kid", parentPageId: "parent", hasChildren: false },
|
||||
];
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
const raw = await readBody(req);
|
||||
if (handleLogin(req, res)) return;
|
||||
if (req.url === "/api/pages/tree") {
|
||||
treeBody = JSON.parse(raw || "{}");
|
||||
sendJson(res, 200, { success: true, data: { items: NODES } });
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/info") {
|
||||
// If checkNewComments still fetched the parent separately this would fire.
|
||||
pageInfoRequests++;
|
||||
sendJson(res, 200, { success: true, data: { id: "parent" } });
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/comments") {
|
||||
const body = JSON.parse(raw || "{}");
|
||||
commentedPages.push(body.pageId);
|
||||
// One fresh comment on the parent, none elsewhere.
|
||||
const items =
|
||||
body.pageId === "parent"
|
||||
? [{ id: "cm1", createdAt: "2030-01-01T00:00:00.000Z", content: null }]
|
||||
: [];
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items, meta: { nextCursor: null } },
|
||||
});
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, {});
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const result = await client.checkNewComments(
|
||||
"space-1",
|
||||
"2020-01-01T00:00:00.000Z",
|
||||
"parent",
|
||||
);
|
||||
|
||||
assert.equal(pageInfoRequests, 0, "no separate getPageRaw for the root");
|
||||
assert.deepEqual(treeBody, { pageId: "parent" }, "subtree scope posts pageId");
|
||||
assert.ok(
|
||||
commentedPages.includes("parent"),
|
||||
"the root itself is in scope (comments fetched for it)",
|
||||
);
|
||||
assert.ok(commentedPages.includes("kid"), "descendants are in scope too");
|
||||
assert.equal(result.checkedPages, 2, "root + one descendant scanned");
|
||||
assert.equal(result.totalNewComments, 1, "the root's fresh comment found");
|
||||
});
|
||||
@@ -297,12 +297,12 @@ test("a response with ONLY authTokenRefresh (no authToken) rejects login", async
|
||||
// -----------------------------------------------------------------------------
|
||||
// 5) paginateAll loop guards.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", async () => {
|
||||
test("paginateAll stops at the MAX_PAGES cap when the server always issues a fresh cursor", async () => {
|
||||
let pageRequests = 0;
|
||||
const LIMIT = 100;
|
||||
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
await readBody(req);
|
||||
const body = JSON.parse((await readBody(req)) || "{}");
|
||||
if (req.url === "/api/auth/login") {
|
||||
sendJson(res, 200, { success: true }, {
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
@@ -311,15 +311,18 @@ test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", a
|
||||
}
|
||||
if (req.url === "/api/spaces") {
|
||||
pageRequests++;
|
||||
// Always return a FULL page (== requested limit) AND hasNextPage:true.
|
||||
// Both the page-length check and the hasNextPage flag say "keep going",
|
||||
// so only the MAX_PAGES ceiling can stop the loop.
|
||||
// Always return a FULL page AND hasNextPage:true with a FRESH nextCursor
|
||||
// that differs from the one the client just sent, so the immovable-cursor
|
||||
// guard never trips — only the MAX_PAGES ceiling can stop the loop.
|
||||
const items = Array.from({ length: LIMIT }, (_, i) => ({
|
||||
id: `s-${pageRequests}-${i}`,
|
||||
}));
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items, meta: { hasNextPage: true } },
|
||||
data: {
|
||||
items,
|
||||
meta: { hasNextPage: true, nextCursor: `cursor-${pageRequests}` },
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -338,7 +341,7 @@ test("paginateAll stops at the MAX_PAGES cap when hasNextPage is always true", a
|
||||
assert.equal(all.length, 50 * LIMIT, "accumulates one full page per request");
|
||||
});
|
||||
|
||||
test("paginateAll stops early on a short page even if hasNextPage is true", async () => {
|
||||
test("paginateAll stops on the immovable-cursor guard when the server ignores the cursor param", async () => {
|
||||
let pageRequests = 0;
|
||||
const LIMIT = 100;
|
||||
|
||||
@@ -352,15 +355,17 @@ test("paginateAll stops early on a short page even if hasNextPage is true", asyn
|
||||
}
|
||||
if (req.url === "/api/spaces") {
|
||||
pageRequests++;
|
||||
// First page is full; second page is SHORT (fewer than limit). The short
|
||||
// page must stop the loop immediately even though hasNextPage stays true.
|
||||
const count = pageRequests === 1 ? LIMIT : 3;
|
||||
const items = Array.from({ length: count }, (_, i) => ({
|
||||
id: `s-${pageRequests}-${i}`,
|
||||
}));
|
||||
// The bug class: the server IGNORES the pagination param and keeps
|
||||
// returning page one with hasNextPage:true and the SAME nextCursor. The
|
||||
// immovable-cursor guard must stop the loop instead of spinning to
|
||||
// MAX_PAGES and duplicating items.
|
||||
const items = Array.from({ length: LIMIT }, (_, i) => ({ id: `s-${i}` }));
|
||||
sendJson(res, 200, {
|
||||
success: true,
|
||||
data: { items, meta: { hasNextPage: true } },
|
||||
data: {
|
||||
items,
|
||||
meta: { hasNextPage: true, nextCursor: "stuck" },
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -370,8 +375,10 @@ test("paginateAll stops early on a short page even if hasNextPage is true", asyn
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
const all = await client.paginateAll("/spaces", {}, LIMIT);
|
||||
|
||||
assert.equal(pageRequests, 2, "stops right after the first short page");
|
||||
assert.equal(all.length, LIMIT + 3, "full page + short page accumulated");
|
||||
// Request 1 sends no cursor and receives "stuck"; request 2 sends "stuck" and
|
||||
// receives "stuck" again -> guard trips after exactly two requests, no dups.
|
||||
assert.equal(pageRequests, 2, "stops once the cursor stops moving");
|
||||
assert.equal(all.length, 2 * LIMIT, "no runaway accumulation past the guard");
|
||||
});
|
||||
|
||||
test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes", async () => {
|
||||
@@ -387,16 +394,16 @@ test("paginateAll handles both {data:{items,meta}} and {items,meta} envelopes",
|
||||
}
|
||||
if (req.url === "/api/groups") {
|
||||
bareRequests.push(1);
|
||||
// Page 1: full page, hasNextPage true. Page 2: short page -> stop.
|
||||
// Page 1: hasNextPage true with a next cursor. Page 2: no next -> stop.
|
||||
if (bareRequests.length === 1) {
|
||||
sendJson(res, 200, {
|
||||
items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })),
|
||||
meta: { hasNextPage: true },
|
||||
meta: { hasNextPage: true, nextCursor: "c2" },
|
||||
});
|
||||
} else {
|
||||
sendJson(res, 200, {
|
||||
items: [{ id: "tail" }],
|
||||
meta: { hasNextPage: false },
|
||||
meta: { hasNextPage: false, nextCursor: null },
|
||||
});
|
||||
}
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user