Compare commits

..

3 Commits

Author SHA1 Message Date
agent_coder 15a9eba562 fix(delivery): immutable-кэш ассетов — отключить дефолтный cacheControl @fastify/static (#452)
Хэшированные ассеты отдавали 'cache-control: public, max-age=0' вместо
'public, max-age=31536000, immutable' → повторные заходы ревалидировали каждый
ассет (десятки 304 × мобильный RTT), главный выигрыш #346 не реализовывался.
Причина: у @fastify/static опция cacheControl:true по умолчанию пишет свой
Cache-Control (из maxAge, дефолт 0) ПОСЛЕ setHeaders-колбэка, затирая immutable-
заголовок из resolveStaticAssetHeaders. Фикс — cacheControl:false, колбэк
владеет заголовком. preCompressed не конфликтовал, потому баг был только в
заголовках.

Крайние случаи проверены: locales/vad/иконки получают только vary (без
cache-control → браузер ревалидирует по etag — ок); index.html отдаётся
отдельным wildcard-роутом со своим no-cache (не затронут); preCompressed .br
получает путь с /assets/ → маппинг матчит, immutable ставится.

Тест: bare-fastify + inject() — /assets/<hashed>.js содержит immutable+
max-age=31536000, /locales/en.json — нет. Мутационно: cacheControl:true роняет
ассерт immutable. jest static.module → 5/5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 07:30:44 +03:00
vvzvlad f8d37d8956 Merge pull request 'fix(mcp): курсорная пагинация — устранить тихую потерю страниц в list_pages/check_new_comments (#442)' (#451) from fix/442-cursor-pagination into develop
Reviewed-on: #451
2026-07-10 07:27:31 +03:00
agent_coder 90168eb926 fix(mcp): курсорная пагинация вместо офсетной — устранить тихую потерю страниц (#442)
Апстрим 78b1c1a4 перевёл серверные эндпоинты на КУРСОРНУЮ пагинацию, а
ValidationPipe({whitelist:true}) молча вырезает неизвестное поле `page`.
MCP-клиент так и слал офсетный `page` → сервер отдавал ту же первую двадцатку
с hasNextPage:true, цикл выкручивался до MAX_PAGES=50 одинаковых запросов, дети
№21+ не выгружались (с поддеревьями). Дедуп `visited` гасил дубли → «дырявое»
дерево без ошибок. Netmap: 20/299 страниц терялось, 160 запросов вместо 62.

- A: enumerateSpacePages → один POST /pages/tree (весь спейс/поддерево разом);
  fallback на курсорный BFS при 404/405 (stock upstream). Возврат {pages,
  truncated}; truncated честный — true только при реальном упоре fallback-BFS в
  MAX_NODES.
- B: listSidebarPages → курсорный цикл, limit:100, guard на неподвижный курсор
  (!next || next===cursor → break) — если протокол снова разойдётся, не крутит
  дубли молча; warn при упоре в MAX_PAGES.
- C: paginateAll (/spaces, /shares) → та же курсорная миграция + guard.
- D: check_new_comments — /pages/tree поддерева включает корень
  (getPageAndDescendants), убран лишний getPageRaw; в fallback корень
  засевается явно (иначе его комменты терялись — регрессия того же класса).
- listComments: do/while → for с MAX_PAGES + guard неподвижного курсора
  (был безлимитный — тот же сценарий #442 дал бы бесконечный цикл).

Внутренний цикл: 2 прохода. Первый нашёл потерю комментов корня в fallback
поддерева (data-loss) → засев корня; догрёб honest-truncated, warn в
listSidebarPages, guard в listComments. Второй проход — APPROVE, форма возврата
{pages,truncated} распространена на оба вызова без пропусков.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 07:08:56 +03:00
6 changed files with 713 additions and 99 deletions
@@ -1,3 +1,8 @@
import * as fs from 'node:fs';
import * as os from 'node:os';
import { join } from 'node:path';
import Fastify, { FastifyInstance } from 'fastify';
import fastifyStatic from '@fastify/static';
import { resolveStaticAssetHeaders } from './static.module'; import { resolveStaticAssetHeaders } from './static.module';
// Unit tests for the static-asset cache classifier extracted from the // Unit tests for the static-asset cache classifier extracted from the
@@ -33,3 +38,69 @@ describe('resolveStaticAssetHeaders', () => {
expect(headers['vary']).toBe('Accept-Encoding'); expect(headers['vary']).toBe('Accept-Encoding');
}); });
}); });
// Integration test proving the ACTUAL response header emitted by @fastify/static
// with the exact registration options StaticModule uses. This is the regression
// guard for #452: without `cacheControl: false`, @fastify/static writes its own
// `Cache-Control: public, max-age=0` AFTER the setHeaders callback, overwriting
// the immutable header — the /assets/ assertion below would then fail.
describe('static.module @fastify/static registration (integration)', () => {
let app: FastifyInstance;
let tmpDir: string;
beforeAll(async () => {
tmpDir = fs.mkdtempSync(join(os.tmpdir(), 'static-module-spec-'));
fs.mkdirSync(join(tmpDir, 'assets'), { recursive: true });
fs.mkdirSync(join(tmpDir, 'locales'), { recursive: true });
fs.writeFileSync(
join(tmpDir, 'assets', 'index-a1b2c3.js'),
'console.log(1);',
);
fs.writeFileSync(join(tmpDir, 'locales', 'en.json'), '{"hello":"world"}');
app = Fastify();
// Mirror StaticModule.onModuleInit's registration options exactly.
await app.register(fastifyStatic, {
root: tmpDir,
wildcard: false,
preCompressed: true,
cacheControl: false,
setHeaders: (res, filePath) => {
for (const [name, value] of Object.entries(
resolveStaticAssetHeaders(filePath),
)) {
res.setHeader(name, value);
}
},
});
await app.ready();
});
afterAll(async () => {
await app.close();
fs.rmSync(tmpDir, { recursive: true, force: true });
});
it('serves a hashed /assets/ file with an immutable, 1-year cache-control', async () => {
const res = await app.inject({
method: 'GET',
url: '/assets/index-a1b2c3.js',
});
expect(res.statusCode).toBe(200);
const cacheControl = res.headers['cache-control'];
expect(cacheControl).toContain('immutable');
expect(cacheControl).toContain('max-age=31536000');
});
it('serves a non-hashed /locales/ file WITHOUT an immutable cache-control', async () => {
const res = await app.inject({ method: 'GET', url: '/locales/en.json' });
expect(res.statusCode).toBe(200);
// resolveStaticAssetHeaders sets no cache-control here and cacheControl:false
// stops @fastify/static from adding one, so the browser revalidates by
// etag/last-modified — either an absent header or one without `immutable`.
const cacheControl = res.headers['cache-control'];
if (cacheControl !== undefined) {
expect(cacheControl).not.toContain('immutable');
}
});
});
@@ -115,6 +115,11 @@ export class StaticModule implements OnModuleInit {
// Serve the build-time .br/.gz neighbour when the client accepts it // Serve the build-time .br/.gz neighbour when the client accepts it
// (see vite-plugin-compression2 in apps/client/vite.config.ts). // (see vite-plugin-compression2 in apps/client/vite.config.ts).
preCompressed: true, preCompressed: true,
// @fastify/static's default cacheControl:true writes its own
// Cache-Control (from maxAge, default 0) AFTER the setHeaders callback,
// silently overwriting the immutable header that resolveStaticAssetHeaders
// sets — disable it so setHeaders/resolveStaticAssetHeaders own the header.
cacheControl: false,
setHeaders: (res, filePath) => { setHeaders: (res, filePath) => {
for (const [name, value] of Object.entries( for (const [name, value] of Object.entries(
resolveStaticAssetHeaders(filePath), resolveStaticAssetHeaders(filePath),
+169 -78
View File
@@ -553,16 +553,18 @@ export class DocmostClient {
// forever and accumulate duplicates). // forever and accumulate duplicates).
const MAX_PAGES = 50; const MAX_PAGES = 50;
let page = 1; let cursor: string | undefined;
let allItems: T[] = []; let allItems: T[] = [];
let hasNextPage = true; let truncated = false;
while (hasNextPage && page <= MAX_PAGES) { for (let page = 0; page < MAX_PAGES; page++) {
const response = await this.client.post(endpoint, { const payload: Record<string, any> = {
...basePayload, ...basePayload,
limit: clampedLimit, limit: clampedLimit,
page, };
}); if (cursor) payload.cursor = cursor;
const response = await this.client.post(endpoint, payload);
const data = response.data; const data = response.data;
const items = data.data?.items || data.items || []; const items = data.data?.items || data.items || [];
@@ -570,22 +572,28 @@ export class DocmostClient {
allItems = allItems.concat(items); allItems = allItems.concat(items);
// Stop if the page is empty or shorter than the requested size: a full // Advance strictly via the server-issued cursor. A missing nextCursor (or
// page worth of items is the only situation where another page can exist, // hasNextPage false) means we reached the end. A cursor identical to the
// so this defends against a stuck hasNextPage flag in addition to it. // one we just sent means the server did not understand our pagination
if (items.length === 0 || items.length < clampedLimit) { // 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; break;
} }
cursor = next;
hasNextPage = meta?.hasNextPage || false; // Reaching the ceiling with more pages still available means the result
page++; // set is truncated.
if (page === MAX_PAGES - 1) truncated = true;
} }
// If the loop stopped because it hit the MAX_PAGES ceiling while the server // 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 // still reported more results, the result set is truncated — warn so the
// full), the result set is truncated — warn so the caller is not silently // caller is not silently handed an incomplete list.
// handed an incomplete list. if (truncated) {
if (hasNextPage && page > MAX_PAGES) {
console.warn( console.warn(
`paginateAll: results from "${endpoint}" truncated at the ${MAX_PAGES}-page cap; more pages exist on the server`, `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 * 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 * 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. * scoped to one space) and IGNORES `limit` — the whole hierarchy is returned.
* It walks the sidebar tree via `enumerateSpacePages`, which performs N * It fetches the tree via `enumerateSpacePages`, which on the fork server
* sidebar requests and is bounded by that method's 10000-node cap (and skips * resolves to a single `/pages/tree` request returning the whole
* soft-deleted pages server-side). * permission-filtered flat page set (soft-deleted pages excluded
* server-side).
*/ */
async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) { async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
await this.ensureAuthenticated(); 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.", "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); const { pages } = await this.enumerateSpacePages(spaceId);
return buildPageTree(nodes); return buildPageTree(pages);
} }
const clampedLimit = Math.max(1, Math.min(100, limit)); const clampedLimit = Math.max(1, Math.min(100, limit));
@@ -655,57 +664,123 @@ export class DocmostClient {
async listSidebarPages(spaceId: string, pageId?: string) { async listSidebarPages(spaceId: string, pageId?: string) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
// Paginate: the endpoint returns server-paged children, so posting only // Paginate via the server-issued cursor. The server switched from OFFSET
// { page: 1 } silently dropped every child beyond the first page. Loop on // (`page`) to CURSOR (`cursor`/`nextCursor`) pagination, and the global
// meta.hasNextPage (with a MAX_PAGES ceiling like paginateAll, guarding // ValidationPipe(whitelist:true) SILENTLY STRIPS the obsolete `page` field
// against a stuck hasNextPage flag) and accumulate all children. // — 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; const MAX_PAGES = 50;
let page = 1; let cursor: string | undefined;
let allItems: any[] = []; 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. // 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 (pageId) payload.pageId = pageId;
if (cursor) payload.cursor = cursor;
const response = await this.client.post("/pages/sidebar-pages", payload); const data = (await this.client.post("/pages/sidebar-pages", payload)).data
const data = response.data?.data ?? response.data; ?.data;
const items = data?.items || []; allItems = allItems.concat(data?.items ?? []);
allItems = allItems.concat(items);
hasNextPage = data?.meta?.hasNextPage || false; // Advance strictly via the server-issued cursor; a missing/repeated cursor
page++; // 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; return allItems;
} }
/** /**
* Enumerate EVERY page in a space (or in a subtree, when rootPageId is given) * Enumerate EVERY page in a space (or in a subtree, when rootPageId is given).
* by walking the sidebar-pages tree.
* *
* Starting set: the children of rootPageId when provided, otherwise the * Primary path (fork server): a SINGLE `POST /pages/tree` returns the whole
* space root pages. From there it does an iterative breadth-first walk: each * space (or a subtree) as a flat, permission-filtered list in one request, in
* node is collected, and when node.hasChildren is true its direct children * the exact node shape buildPageTree consumes. This replaces the old
* are fetched via listSidebarPages(spaceId, node.id) and enqueued. * 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 * The subtree variant (rootPageId given) INCLUDES the root node itself
* recent-activity feed (~5000 cap) and therefore misses comments on older * (getPageAndDescendants seeds with id = rootPageId), unlike the old BFS
* pages that were never recently touched. * which started from the root's children.
* *
* Safeguards: a `visited` Set of page ids prevents re-processing a node * Fallback path (stdio mode may target STOCK upstream Docmost, which lacks
* (cycles / duplicate references), and a hard node cap bounds pathological * `/pages/tree`): on a 404/405 it falls back to the cursor-based BFS below,
* trees so the walk always terminates. * 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( private async enumerateSpacePages(
spaceId: string, spaceId: string,
rootPageId?: 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 MAX_NODES = 10000;
const result: any[] = []; const result: any[] = [];
const visited = new Set<string>(); 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). // Seed the queue with the starting level (subtree children or roots).
const queue: any[] = await this.listSidebarPages(spaceId, rootPageId); 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. */ /** Raw page info including the ProseMirror JSON content and slugId. */
@@ -2364,7 +2444,13 @@ export class DocmostClient {
let allComments: any[] = []; let allComments: any[] = [];
let cursor: string | null = null; 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 }; const payload: Record<string, any> = { pageId, limit: 100 };
if (cursor) payload.cursor = cursor; if (cursor) payload.cursor = cursor;
@@ -2372,8 +2458,23 @@ export class DocmostClient {
const data = response.data.data || response.data; const data = response.data.data || response.data;
const items = data.items || []; const items = data.items || [];
allComments = allComments.concat(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 mapped = allComments.map((comment: any) => {
const markdown = comment.content 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 // 1. Enumerate the FULL set of pages in scope via the page tree (a complete
// tree (a complete page index), NOT the bounded "/pages/recent" feed which // page index), NOT the bounded "/pages/recent" feed which caps at ~5000
// caps at ~5000 recent items and silently misses comments on older pages. // recent items and silently misses comments on older pages.
// //
// Subtree scope: when parentPageId is given, the scope is that page ITSELF // Subtree scope: when parentPageId is given, the scope is that page ITSELF
// plus every descendant (enumerateSpacePages walks its children). Otherwise // plus every descendant. Otherwise the scope is the whole space (all roots
// the scope is the whole space (all roots and their descendants). // and their descendants).
// //
// NOTE: do NOT pre-filter by page.updatedAt — creating a comment does not // 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 // 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 // comments on pages that were not otherwise edited. The complete tree walk
// already restricts the scope correctly, so no recent-feed allow-list is // already restricts the scope correctly, so no recent-feed allow-list is
// needed any more. // needed any more.
let pagesInScope: any[]; //
if (parentPageId) { // The subtree scope (parentPageId given) already INCLUDES the root node
const subtree = await this.enumerateSpacePages(spaceId, parentPageId); // itself: /pages/tree seeds getPageAndDescendants with id = parentPageId, so
// Include the parent page node itself alongside its descendants. Fetch it // no separate getPageRaw fetch for the parent is needed.
// so its title/id are available even though it is not returned by its own const { pages: pagesInScope, truncated } = await this.enumerateSpacePages(
// children listing. spaceId,
let parentNode: any = { id: parentPageId }; 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);
}
// 2. Fetch comments for each page, keep ones created after since // 2. Fetch comments for each page, keep ones created after since
const results: any[] = []; const results: any[] = [];
@@ -2904,10 +2996,9 @@ export class DocmostClient {
0, 0,
); );
// enumerateSpacePages caps traversal at 10000 nodes; flag when that cap was // `truncated` is reported by enumerateSpacePages: it is true ONLY when the
// hit so the caller knows the scan may be incomplete (some pages skipped). // stdio fallback BFS hit its node cap. The primary /pages/tree path is
const truncated = pagesInScope.length >= 10000; // uncapped, so a space with legitimately many pages is not falsely flagged.
return { return {
since, since,
scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`, scope: parentPageId ? `subtree of ${parentPageId}` : `space ${spaceId}`,
@@ -6,7 +6,7 @@
* typographic quotes («…»/“…”) vs ASCII "…", em/en-dash vs `-`, non-breaking * typographic quotes («…»/“…”) vs ASCII "…", em/en-dash vs `-`, non-breaking
* space vs normal space, differing space counts — are not recognized as equal * space vs normal space, differing space counts — are not recognized as equal
* and "fork": two definitions appear where the author meant one. The existing * and "fork": two definitions appear where the author meant one. The existing
* de-dup paths miss this: `footnoteContentKey` (@docmost/prosemirror-markdown) only * de-dup paths miss this: `footnoteContentKey` (footnote-authoring.ts) only
* collapses ASCII whitespace (quotes/dashes/NBSP untouched), and * collapses ASCII whitespace (quotes/dashes/NBSP untouched), and
* `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have * `canonicalizeFootnotes` keys purely by `attrs.id` (the two forks have
* different ids), so neither glues the forks together. * different ids), so neither glues the forks together.
@@ -195,7 +195,7 @@ function stableAttrs(attrs: any): string {
/** /**
* ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from * ATTRS-AWARE merge key for a footnote definition. Deliberately DIVERGES from
* the shared `footnoteContentKey` (@docmost/prosemirror-markdown): that key's mark * the shared `footnoteContentKey` (footnote-authoring.ts): that key's mark
* signature is TYPE-ONLY (`m.type`), so two definitions with identical visible * signature is TYPE-ONLY (`m.type`), so two definitions with identical visible
* text but marks differing only in ATTRIBUTES — most importantly a `link` with a * text but marks differing only in ATTRIBUTES — most importantly a `link` with a
* different `href` (footnotes are usually citations/links), also `code` / * different `href` (footnotes are usually citations/links), also `code` /
@@ -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");
});
+26 -19
View File
@@ -297,12 +297,12 @@ test("a response with ONLY authTokenRefresh (no authToken) rejects login", async
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------
// 5) paginateAll loop guards. // 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; let pageRequests = 0;
const LIMIT = 100; const LIMIT = 100;
const { baseURL } = await spawn(async (req, res) => { const { baseURL } = await spawn(async (req, res) => {
await readBody(req); const body = JSON.parse((await readBody(req)) || "{}");
if (req.url === "/api/auth/login") { if (req.url === "/api/auth/login") {
sendJson(res, 200, { success: true }, { sendJson(res, 200, { success: true }, {
"Set-Cookie": "authToken=t; Path=/; HttpOnly", "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") { if (req.url === "/api/spaces") {
pageRequests++; pageRequests++;
// Always return a FULL page (== requested limit) AND hasNextPage:true. // Always return a FULL page AND hasNextPage:true with a FRESH nextCursor
// Both the page-length check and the hasNextPage flag say "keep going", // that differs from the one the client just sent, so the immovable-cursor
// so only the MAX_PAGES ceiling can stop the loop. // guard never trips — only the MAX_PAGES ceiling can stop the loop.
const items = Array.from({ length: LIMIT }, (_, i) => ({ const items = Array.from({ length: LIMIT }, (_, i) => ({
id: `s-${pageRequests}-${i}`, id: `s-${pageRequests}-${i}`,
})); }));
sendJson(res, 200, { sendJson(res, 200, {
success: true, success: true,
data: { items, meta: { hasNextPage: true } }, data: {
items,
meta: { hasNextPage: true, nextCursor: `cursor-${pageRequests}` },
},
}); });
return; 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"); 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; let pageRequests = 0;
const LIMIT = 100; 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") { if (req.url === "/api/spaces") {
pageRequests++; pageRequests++;
// First page is full; second page is SHORT (fewer than limit). The short // The bug class: the server IGNORES the pagination param and keeps
// page must stop the loop immediately even though hasNextPage stays true. // returning page one with hasNextPage:true and the SAME nextCursor. The
const count = pageRequests === 1 ? LIMIT : 3; // immovable-cursor guard must stop the loop instead of spinning to
const items = Array.from({ length: count }, (_, i) => ({ // MAX_PAGES and duplicating items.
id: `s-${pageRequests}-${i}`, const items = Array.from({ length: LIMIT }, (_, i) => ({ id: `s-${i}` }));
}));
sendJson(res, 200, { sendJson(res, 200, {
success: true, success: true,
data: { items, meta: { hasNextPage: true } }, data: {
items,
meta: { hasNextPage: true, nextCursor: "stuck" },
},
}); });
return; 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 client = new DocmostClient(baseURL, "user@example.com", "pw");
const all = await client.paginateAll("/spaces", {}, LIMIT); const all = await client.paginateAll("/spaces", {}, LIMIT);
assert.equal(pageRequests, 2, "stops right after the first short page"); // Request 1 sends no cursor and receives "stuck"; request 2 sends "stuck" and
assert.equal(all.length, LIMIT + 3, "full page + short page accumulated"); // 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 () => { 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") { if (req.url === "/api/groups") {
bareRequests.push(1); 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) { if (bareRequests.length === 1) {
sendJson(res, 200, { sendJson(res, 200, {
items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })), items: Array.from({ length: 100 }, (_, i) => ({ id: `g${i}` })),
meta: { hasNextPage: true }, meta: { hasNextPage: true, nextCursor: "c2" },
}); });
} else { } else {
sendJson(res, 200, { sendJson(res, 200, {
items: [{ id: "tail" }], items: [{ id: "tail" }],
meta: { hasNextPage: false }, meta: { hasNextPage: false, nextCursor: null },
}); });
} }
return; return;