fix(mcp): курсорная пагинация — устранить тихую потерю страниц в list_pages/check_new_comments (#442) #451

Merged
vvzvlad merged 1 commits from fix/442-cursor-pagination into develop 2026-07-10 07:27:34 +03:00
Collaborator

Summary

Data-loss фикс: list_pages tree:true молча терял страницы и жёг до 50 холостых запросов на «широкого» родителя. Апстрим 78b1c1a4 перевёл серверные эндпоинты на КУРСОРНУЮ пагинацию, а глобальный ValidationPipe({whitelist:true}) молча вырезает неизвестное поле page. MCP-клиент так и слал офсетный page → сервер отдавал ту же первую двадцатку с hasNextPage:true, цикл выкручивался до MAX_PAGES=50 одинаковых запросов, дети №21+ не выгружались вместе с поддеревьями; дедуп visited гасил дубли → «дырявое» дерево без единой ошибки. Netmap: 20/299 страниц терялось, 160 запросов вместо 62.

closes #442

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

Эффект: Netmap 160 запросов → 1, потерь нет.

How verified

Прогон на стенде (node --test):

  • pnpm --filter @docmost/mcp build → чисто (tsc).
  • node --test test/mock/pagination-cursor.test.mjs test/mock/reauth.test.mjs17/17 passed: курсорный цикл собирает все элементы (кол-во запросов = кол-ву страниц); regression-тест класса бага — сервер игнорирует курсор → цикл завершается за 1 лишнюю итерацию без дублей (не крутит до MAX_PAGES); happy-path /pages/tree одним запросом; fallback на 404; fallback поддерева засевает корень (проверено пойманным /pages/info + /api/comments на root); listComments immovable-cursor → 2 запроса без дублей.
  • 2 предсуществующих падения markdown-converter.test.mjs (отступы вложенных списков) — ортогональны, подтверждено на базе.

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

Checklist

  • критерии приёмки из #442 выполнены (list_pages/check_new_comments/get_page/getSpaces/list_shares; ни один цикл не крутит один и тот же курсор молча)
  • вне заявленного scope ничего не менялось
## Summary Data-loss фикс: `list_pages tree:true` молча терял страницы и жёг до 50 холостых запросов на «широкого» родителя. Апстрим `78b1c1a4` перевёл серверные эндпоинты на КУРСОРНУЮ пагинацию, а глобальный `ValidationPipe({whitelist:true})` молча вырезает неизвестное поле `page`. MCP-клиент так и слал офсетный `page` → сервер отдавал ту же первую двадцатку с `hasNextPage:true`, цикл выкручивался до `MAX_PAGES=50` одинаковых запросов, дети №21+ не выгружались вместе с поддеревьями; дедуп `visited` гасил дубли → «дырявое» дерево без единой ошибки. Netmap: **20/299** страниц терялось, **160** запросов вместо 62. closes #442 - **A** `enumerateSpacePages` → один `POST /pages/tree` (весь спейс/поддерево разом, permission-filtered); fallback на курсорный BFS при 404/405 (stock upstream без форкового эндпоинта). Возврат `{pages, truncated}`; `truncated` честный — `true` только при реальном упоре fallback-BFS в `MAX_NODES` (primary-путь без cap → всегда `false`). - **B** `listSidebarPages` → курсорный цикл, `limit:100`, **guard на неподвижный курсор** (`!next || next===cursor → break`) — если протокол снова разойдётся, цикл не крутит дубли молча; warn при упоре в `MAX_PAGES`. - **C** `paginateAll` (`/spaces`, `/shares`) → та же курсорная миграция + guard + сохранённый truncation-warn. - **D** `check_new_comments` — `/pages/tree` поддерева включает корень (`getPageAndDescendants`), убран лишний `getPageRaw`; в fallback-ветке корень засевается явно (иначе его комментарии терялись — регрессия того же класса). - `listComments`: `do/while` → `for` с `MAX_PAGES` + guard неподвижного курсора (был безлимитный — тот же сценарий #442 дал бы бесконечный цикл). Эффект: Netmap 160 запросов → **1**, потерь нет. ## How verified Прогон на стенде (`node --test`): - `pnpm --filter @docmost/mcp build` → чисто (tsc). - `node --test test/mock/pagination-cursor.test.mjs test/mock/reauth.test.mjs` → **17/17 passed**: курсорный цикл собирает все элементы (кол-во запросов = кол-ву страниц); **regression-тест класса бага** — сервер игнорирует курсор → цикл завершается за 1 лишнюю итерацию без дублей (не крутит до MAX_PAGES); happy-path `/pages/tree` одним запросом; fallback на 404; **fallback поддерева засевает корень** (проверено пойманным `/pages/info` + `/api/comments` на root); `listComments` immovable-cursor → 2 запроса без дублей. - 2 предсуществующих падения `markdown-converter.test.mjs` (отступы вложенных списков) — ортогональны, подтверждено на базе. Внутренний цикл: 2 прохода. Первый нашёл потерю комментариев корня в fallback поддерева (data-loss) → засев корня; догрёб honest-`truncated` (убрал ложную эвристику `>=10000` на primary-пути), warn в `listSidebarPages`, guard в `listComments`. Второй проход — APPROVE: форма возврата `{pages,truncated}` распространена на оба вызова без пропусков, семантика `truncated` честна end-to-end. ## Checklist - [x] критерии приёмки из #442 выполнены (list_pages/check_new_comments/get_page/getSpaces/list_shares; ни один цикл не крутит один и тот же курсор молча) - [x] вне заявленного scope ничего не менялось
agent_coder added 1 commit 2026-07-10 07:09:48 +03:00
Апстрим 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>
agent_coder added the review/needs label 2026-07-10 07:09:48 +03:00
Collaborator

Ревью — #451 fix(mcp): курсорная пагинация — устранить тихую потерю страниц в list_pages/check_new_comments (#442)

Вердикт: PASS

Полный веер из 9 аспектов по всему диффу PR (merge-base ae790da1). Гейт зелёный (clean-room /home/claude/gm-ci): mcp build (tsc) 0, node --test 587/587 (pagination-cursor +440).

Корректность пагинации — проверено (клик)
  • Нет тихой потери: офсетная пагинация на мутирующем датасете сдвигала окно → пропуск/дубли. Новый цикл идёт по КУРСОРУ (next = meta.hasNextPage ? nextCursor : null), продвижение завязано на курсор, а не на размер страницы. Все 3 пагинатора (paginateAll/listSidebarPages/listComments) идентичной структуры.
  • Ключевой гард if (!next || next === cursor) break; предотвращает: (а) бесконечный цикл и (б) runaway-дубли при «застрявшем» курсоре (сервер игнорит курсор → break на 2-м запросе, а не спин до MAX_PAGES). Мутационно покрыт (сломать гард → тесты «terminates no dups» падают).
  • Ограничение: MAX_PAGES=50 + флаг truncated (взводится только при упоре в потолок с продвижением). enumerateSpacePages: первичный /pages/tree (без капа) + BFS-фолбэк с seeding root через getPageRaw на 404. checkNewComments берёт {pages, truncated} напрямую.
  • Тесты: walks-all-cursor-pages (collects every item), immovable-guard ×2, tree/BFS/root-seeding, reauth mid-pagination. Security: курсор выдаётся сервером (не инъекция), read-only. Architecture/conventions/documentation — LGTM.
DROP — кодеру НЕ делать (лог калибровки)

Клиентская логика корректна и её ядро (курсор-цикл + immovable-гард) мутационно-покрыто; остаток — redundant/тривиально:

  • [below-threshold] med [test-coverage] нет теста «мутация датасета mid-pagination → нет пропуска/дубля» (ревьюер оценил critical). Мисскоуп для КЛИЕНТА: mcp-клиент следует курсору сервера ИДЕНТИЧНО, мутировал датасет или нет — иммунитет к mutation-loss это СЕРВЕРНОЕ свойство (стабильный keyset), вне зоны mcp-клиента; клиентский mutation-тест дублирует уже покрытый static-walk (collects-every-item). Ответственность клиента (обойти страницы, собрать всё, гардить кривой сервер) — покрыта.
  • [below-threshold] low [test-coverage] нет отдельных тестов пустого результата и одностраничного случая — тривиальные пути, обрабатываются уже покрытым гардом !next (correctness-агент это подтвердил). Если хочешь для полноты — дёшево добавить, но не отдельный раунд.
  • [below-threshold] low [simplification] гард if (!next || next === cursor) break; дублируется в 3 функциях — 2-3 строки, можно вынести в shouldStopCursorPagination(), но не критично.
  • [by-design] low [coherence] убран fallback items.length < limit для last-page — намеренно и корректно (сервер = источник истины для курсора), immovable-гард страхует протокол-дрейф.

DO/ESCALATE нет. Курсорная пагинация корректна (продвижение по курсору, гард от потери/дублей/бесконечного цикла мутационно-держится, MAX_PAGES+truncated), гейт зелёный. Тихая потеря устранена. Approve.

9 аспектов. Корректность цикла (нет потери/дублей/инфинит-лупа) + immovable-гард-мутация проверены; mutation-scenario тест — мисскоуп (серверный keyset), edge-кейсы тривиальны → DROP с обоснованием (как #364/#439). Escalate нет.

<!-- state:review reviewed_head=90168eb9 round=1 verdict=approved --> ## Ревью — #451 `fix(mcp): курсорная пагинация — устранить тихую потерю страниц в list_pages/check_new_comments (#442)` **Вердикт: PASS** ✅ Полный веер из 9 аспектов по всему диффу PR (merge-base `ae790da1`). Гейт зелёный (clean-room `/home/claude/gm-ci`): mcp build (tsc) 0, **node --test 587/587** (pagination-cursor +440). <details> <summary>Корректность пагинации — проверено (клик)</summary> - **Нет тихой потери**: офсетная пагинация на мутирующем датасете сдвигала окно → пропуск/дубли. Новый цикл идёт по КУРСОРУ (`next = meta.hasNextPage ? nextCursor : null`), продвижение завязано на курсор, а не на размер страницы. Все 3 пагинатора (`paginateAll`/`listSidebarPages`/`listComments`) идентичной структуры. - **Ключевой гард** `if (!next || next === cursor) break;` предотвращает: (а) бесконечный цикл и (б) runaway-дубли при «застрявшем» курсоре (сервер игнорит курсор → break на 2-м запросе, а не спин до MAX_PAGES). **Мутационно покрыт** (сломать гард → тесты «terminates no dups» падают). - **Ограничение**: MAX_PAGES=50 + флаг `truncated` (взводится только при упоре в потолок с продвижением). `enumerateSpacePages`: первичный `/pages/tree` (без капа) + BFS-фолбэк с seeding root через getPageRaw на 404. `checkNewComments` берёт `{pages, truncated}` напрямую. - Тесты: walks-all-cursor-pages (collects every item), immovable-guard ×2, tree/BFS/root-seeding, reauth mid-pagination. Security: курсор выдаётся сервером (не инъекция), read-only. Architecture/conventions/documentation — LGTM. </details> <details> <summary>⛔ DROP — кодеру НЕ делать (лог калибровки)</summary> Клиентская логика корректна и её ядро (курсор-цикл + immovable-гард) мутационно-покрыто; остаток — redundant/тривиально: - `[below-threshold]` `med` **[test-coverage]** нет теста «мутация датасета mid-pagination → нет пропуска/дубля» (ревьюер оценил critical). Мисскоуп для КЛИЕНТА: mcp-клиент следует курсору сервера ИДЕНТИЧНО, мутировал датасет или нет — иммунитет к mutation-loss это СЕРВЕРНОЕ свойство (стабильный keyset), вне зоны mcp-клиента; клиентский mutation-тест дублирует уже покрытый static-walk (collects-every-item). Ответственность клиента (обойти страницы, собрать всё, гардить кривой сервер) — покрыта. - `[below-threshold]` `low` **[test-coverage]** нет отдельных тестов пустого результата и одностраничного случая — тривиальные пути, обрабатываются уже покрытым гардом `!next` (correctness-агент это подтвердил). Если хочешь для полноты — дёшево добавить, но не отдельный раунд. - `[below-threshold]` `low` **[simplification]** гард `if (!next || next === cursor) break;` дублируется в 3 функциях — 2-3 строки, можно вынести в `shouldStopCursorPagination()`, но не критично. - `[by-design]` `low` **[coherence]** убран fallback `items.length < limit` для last-page — намеренно и корректно (сервер = источник истины для курсора), immovable-гард страхует протокол-дрейф. </details> DO/ESCALATE нет. Курсорная пагинация корректна (продвижение по курсору, гард от потери/дублей/бесконечного цикла мутационно-держится, MAX_PAGES+truncated), гейт зелёный. Тихая потеря устранена. Approve. <sub>9 аспектов. Корректность цикла (нет потери/дублей/инфинит-лупа) + immovable-гард-мутация проверены; mutation-scenario тест — мисскоуп (серверный keyset), edge-кейсы тривиальны → DROP с обоснованием (как #364/#439). Escalate нет.</sub>
agent_reviewer added review/approved and removed review/needs labels 2026-07-10 07:26:48 +03:00
vvzvlad merged commit f8d37d8956 into develop 2026-07-10 07:27:34 +03:00
Sign in to join this conversation.