vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(ai-chat,page): дружелюбные ошибки tool-input (#190) + защита от цикла при встречных перемещениях (#159 #9) #200

Closed
vvzvlad wants to merge 3 commits from fix/issues-190-159 into develop
pull from: fix/issues-190-159
merge into: vvzvlad:develop
Conversation 1 Commits 3 Files Changed 9 +525 -94
vvzvlad commented 2026-06-25 23:41:50 +03:00
Owner
Copy Link

Батч из двух независимых правок на одной ветке (fix/issues-190-159), по одному коммиту на находку.

fix(ai-chat): дружелюбные ошибки валидации tool-input — #190

При параллельной партии tool-call'ов модель иногда роняет обязательный параметр (обычно повторяющийся pageId). zod отклонял ввод, а AI SDK отдавал модели сырой zod-текст (Invalid input: expected string, received undefined) — по нему модель не могла осознанно повторить вызов.

Добавлен централизованный враппер входных схем modelFriendlyInput(shape):

  • JSON-схема для модели выводится из той же zod-формы через z.toJSONSchema(z.object(shape), { target: 'draft-7' }) — required/description/ограничения не меняются (контракт сохранён);
  • при ошибке validate возвращает человекочитаемое сообщение с именем проблемного параметра и подсказкой повторить со всеми обязательными полями;
  • при успехе возвращается распарсенный (очищенный от лишних ключей) объект — это сохраняет существующие strip-гайдрейлы (deletePage не пробрасывает permanentlyDelete/forceDelete; transformPage — deleteComments).

Применён в ai-chat-tools.service.ts (билдер sharedTool + все инлайн-тулы) и public-share-chat-tools.service.ts. Значения не угадываются, контракт required/optional не тронут.

fix(page): защита от цикла при встречных перемещениях — #159 (находка #9)

Гонка двух встречных перемещений (A тянет X под Y, B одновременно Y под X) проходила проверку цикла против устаревшего снимка, и обе записи коммитились → X.parent=Y И Y.parent=X: цикл без пути к корню, рекурсивные ancestor-CTE ломаются, оба поддерева пропадают из сайдбара. Guard и запись не были в транзакции.

Для подлинного переноса под конкретного родителя guard+запись обёрнуты в одну executeTx-транзакцию: строки перемещаемой страницы и целевого родителя блокируются FOR UPDATE в каноническом (по id) порядке (сериализация без дедлока), проверка цикла перевыполняется внутри транзакции (проигравший видит закоммиченный репарент и отклоняется существующей ошибкой «Cannot move a page into its own subtree»). Реордер в том же родителе и перенос в корень идут прежним путём (цикл невозможен). Phantom-broadcast gate и PAGE_MOVED без изменений.

Границы: закрыта двухсторонняя гонка (находка #9). Трёхсторонняя цепочка (A→B, B→C, C→A) — вне рамок, отмечена как известное ограничение. Находка #5 (токенный бюджет) намеренно не делается (депириоритезирована в самом issue).

Тесты

  • #190: новый model-friendly-input.spec.ts (сообщение именует параметр + подсказка; лишние ключи срезаются; JSON-схема сохраняет required/description); два .parse()-теста гайдрейлов переведены на новый validate-путь.
  • #159: юнит — разводка локов (findById с { withLock: true, trx } для обоих id в отсортированном порядке; trx прокинут в getPageBreadCrumbs/updatePage) + сохранены прежние гарантии; интеграционный page-move-cycle-concurrency.int-spec.ts (реальный Postgres в CI): два встречных перемещения дают ровно один успех + один отказ и никогда не сохраняют цикл.

tsc --noEmit чистый; затронутые юнит-наборы зелёные.

Батч из двух независимых правок на одной ветке (`fix/issues-190-159`), по одному коммиту на находку. ## fix(ai-chat): дружелюбные ошибки валидации tool-input — #190 При **параллельной** партии tool-call'ов модель иногда роняет обязательный параметр (обычно повторяющийся `pageId`). zod отклонял ввод, а AI SDK отдавал модели **сырой zod-текст** (`Invalid input: expected string, received undefined`) — по нему модель не могла осознанно повторить вызов. Добавлен централизованный враппер входных схем `modelFriendlyInput(shape)`: - JSON-схема для модели выводится из той же zod-формы через `z.toJSONSchema(z.object(shape), { target: 'draft-7' })` — `required`/`description`/ограничения **не меняются** (контракт сохранён); - при ошибке `validate` возвращает человекочитаемое сообщение с **именем** проблемного параметра и подсказкой повторить со всеми обязательными полями; - при успехе возвращается распарсенный (очищенный от лишних ключей) объект — это сохраняет существующие strip-гайдрейлы (`deletePage` не пробрасывает `permanentlyDelete`/`forceDelete`; `transformPage` — `deleteComments`). Применён в `ai-chat-tools.service.ts` (билдер `sharedTool` + все инлайн-тулы) и `public-share-chat-tools.service.ts`. Значения не угадываются, контракт required/optional не тронут. ## fix(page): защита от цикла при встречных перемещениях — #159 (находка #9) Гонка двух встречных перемещений (A тянет X под Y, B одновременно Y под X) проходила проверку цикла против устаревшего снимка, и обе записи коммитились → `X.parent=Y` И `Y.parent=X`: цикл без пути к корню, рекурсивные ancestor-CTE ломаются, оба поддерева пропадают из сайдбара. Guard и запись не были в транзакции. Для подлинного переноса под конкретного родителя guard+запись обёрнуты в одну `executeTx`-транзакцию: строки перемещаемой страницы и целевого родителя блокируются `FOR UPDATE` в **каноническом (по id) порядке** (сериализация без дедлока), проверка цикла перевыполняется **внутри** транзакции (проигравший видит закоммиченный репарент и отклоняется существующей ошибкой «Cannot move a page into its own subtree»). Реордер в том же родителе и перенос в корень идут прежним путём (цикл невозможен). Phantom-broadcast gate и `PAGE_MOVED` без изменений. _Границы:_ закрыта двухсторонняя гонка (находка #9). Трёхсторонняя цепочка (A→B, B→C, C→A) — вне рамок, отмечена как известное ограничение. Находка #5 (токенный бюджет) намеренно не делается (депириоритезирована в самом issue). ## Тесты - #190: новый `model-friendly-input.spec.ts` (сообщение именует параметр + подсказка; лишние ключи срезаются; JSON-схема сохраняет required/description); два `.parse()`-теста гайдрейлов переведены на новый `validate`-путь. - #159: юнит — разводка локов (`findById` с `{ withLock: true, trx }` для обоих id в отсортированном порядке; trx прокинут в `getPageBreadCrumbs`/`updatePage`) + сохранены прежние гарантии; интеграционный `page-move-cycle-concurrency.int-spec.ts` (реальный Postgres в CI): два встречных перемещения дают ровно один успех + один отказ и **никогда** не сохраняют цикл. `tsc --noEmit` чистый; затронутые юнит-наборы зелёные. --- - Closes #190 - Частично закрывает #159 (находка #9 из 10; зонтичный issue остаётся открыт под находку #5). - #161 сюда **не входит** — он вливается отдельно через PR #162 (`fix/ai-chat-new-chat-during-stream`).
vvzvlad added 2 commits 2026-06-25 23:41:50 +03:00
fix(ai-chat): model-friendly tool-input validation errors (#190) 549fc611aa 
When the in-app AI chat issued a parallel batch of tool calls, the model
sometimes dropped a required parameter (typically a repeated `pageId`). zod
rejected it and the AI SDK surfaced the raw zod text
("Invalid input: expected string, received undefined") to the model, which is
not actionable — so it could not reliably retry.

Add a centralized `modelFriendlyInput(shape)` wrapper for in-app tool input
schemas:
- the model-facing JSON schema is derived from the SAME zod shape via
  `z.toJSONSchema(z.object(shape), { target: 'draft-7' })`, so `required`,
  `description` and field constraints are unchanged (contract preserved);
- on a validation failure `validate` returns a human-readable Error naming the
  offending parameter(s) plus a fixed retry hint ("Include every REQUIRED
  parameter ... do not drop ids like pageId"), which the SDK relays to the model
  via InvalidToolInputError;
- on success it returns the parsed (unknown-key-stripped) data, preserving the
  existing strip guardrails (deletePage never forwards permanentlyDelete/
  forceDelete; transformPage never forwards deleteComments).

Applied in ai-chat-tools.service.ts (the sharedTool builder + every inline
tool inputSchema) and in public-share-chat-tools.service.ts. Values are never
guessed and the required/optional contract is untouched.

Tests: new model-friendly-input.spec.ts (friendly message names the param +
hint; unknown keys stripped; JSON schema keeps required/description); the two
.parse()-based guardrail tests reworked to assert via the new validate path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(page): serialize concurrent moves so opposing re-parents can't form a cycle (#159) 1bbaf84d42 
Red-team finding #9: two opposing moves racing each other — A drags X under Y
while B drags Y under X — each passed a cycle check built from a stale snapshot
and both committed, leaving X.parent=Y AND Y.parent=X. That cycle has no path to
root, so the recursive ancestor CTEs break and both subtrees disappear from the
sidebar. The cycle guard (breadcrumb read) and the parent-update write were not
in a transaction.

For a genuine re-parent under a concrete parent (the only cycle-capable path),
wrap the guard + update in one executeTx transaction and:
- lock the moved page and the destination parent rows FOR UPDATE
  (pageRepo.findById(id, { withLock: true, trx })) in a canonical, id-sorted
  order so the two opposing moves serialize instead of deadlocking;
- re-run the ancestor cycle check INSIDE the transaction, so the loser sees the
  winner's committed re-parent and is rejected with the existing
  "Cannot move a page into its own subtree" error.
Same-parent reorder (parentPageId undefined) and move-to-root (null) keep the
plain non-transactional update — neither can create a cycle. The phantom-
broadcast gate and PAGE_MOVED emit are unchanged. getPageBreadCrumbs gains an
optional trx so its recursive CTE can run inside the locked transaction.

Scope: this closes the two-party opposing-move race (finding #9). A longer
3-party chain (A→B, B→C, C→A) is out of scope and left as a known limitation.

Tests:
- unit (page.service.spec.ts): assert the lock wiring — findById called with
  { withLock: true, trx } for both ids in sorted order, getPageBreadCrumbs and
  updatePage receive the trx — while keeping the self-move / cycle-reject and
  legitimate-move guarantees.
- integration (page-move-cycle-concurrency.int-spec.ts, real Postgres in CI):
  two opposing concurrent moves resolve to exactly one success + one rejection
  and never persist a cycle.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vvzvlad added the bug label 2026-06-26 00:31:08 +03:00
vvzvlad commented 2026-06-26 16:42:23 +03:00
Author
Owner
Copy Link

Code review — PR #200: дружелюбные ошибки tool-input в AI-чате (#190) + guard от циклов при конкурентных перемещениях страниц (#159)

Вердикт: Approve with comments. Логика корректна и безопасна (security/regressions чисты, цикл при гонке re-parent действительно сериализуется через FOR UPDATE + re-check внутри транзакции). Блокирует только одно: по конвенции репозитория обе пользовательские правки должны попасть в CHANGELOG.md [Unreleased], а он не тронут. Остальное — non-blocking.

Объём: дифф develop…fix/issues-190-159 (merge-base 3ddc329b), 8 файлов, +467/−79. Прогнаны параллельные аспектные ревьюеры (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) + judge-проход.

Must fix before merge

  • [documentation] Добавить записи в CHANGELOG.md [Unreleased] для #190 и #159 — CHANGELOG.md:11
    Репозиторий ведёт Keep-a-Changelog с секцией [Unreleased], и недавние PR (#163, #146/#147, #183) последовательно добавляли пункты по каждой пользовательской правке; ревью-коммиты прошлых PR трактовали отсутствие записи как дефект. PR везёт два user-facing изменения (исправление потери поддеревьев из-за цикла при конкурентном перемещении — #159; дружелюбные ошибки валидации tool-input в AI-чате с указанием параметра и retry-подсказкой вместо сырого zod-текста — #190), но CHANGELOG.md в диффе пустой. Fix: под ## [Unreleased] добавить ### Fixed-пункт для #159 (опаздывающие re-parent сериализуются FOR UPDATE с re-check цикла внутри транзакции) и ### Added/### Changed-пункт для #190, в стиле существующих записей с ссылками на (#159) и (#190).

Non-blocking

  • [stability] Брать lock по каноническим UUID (movedPage.id, parentPageId), а не по сырому dto.pageId — apps/server/src/core/page/services/page.service.ts:943-944
    Защита от дедлока держится на том, что оба встречных перемещения сортируют ОДНИ И ТЕ ЖЕ две строки-идентификатора и берут FOR UPDATE в одинаковом порядке. Сейчас const lockIds = [dto.pageId, parentPageId].sort(): parentPageId — уже разрешённый UUID (parentPage.id), а dto.pageId — сырое клиентское значение, и findById принимает также slugId (MovePageDto.pageId — неограниченный @IsString). Sidebar всегда шлёт UUID, так что на практике безопасно, но REST/agent-клиент, передавший slugId в двух одновременных встречных перемещениях (A: X под Y, B: Y под X), даст противоположный физический порядок блокировок и воспроизведёт AB-BA дедлок, который Postgres разрешит, отдав 500 вместо дружелюбного BadRequestException. Повреждения данных нет (цикл всё равно предотвращается), но контракт «не может задедлочиться» из комментария нарушается для slug-входа. Fix: const lockIds = [movedPage.id, parentPageId].sort(); — movedPage.id это UUID, разрешённый контроллером, parentPageId уже UUID; dto.pageId для breadcrumb-check / updatePage оставить как есть (та же строка в транзакции).

  • [simplification] Вынести дублированный stub makeChain (Proxy для chainable-trx) в один модульный хелпер — apps/server/src/core/page/services/page.service.spec.ts:46-56,327-337
    PR добавляет две байт-в-байт копии Proxy-стаба makeChain поверх уже существующей в блоке movePageToSpace() на develop — три идентичные копии ~10-строчного хелпера в одном файле, каждую правку trx-стаба придётся делать в трёх местах (сами комментарии диффа отмечают, что «mirror the pattern»). Fix: завести один makeChain()/makeTrxStub() на уровне describe PageService (или модуля) и переиспользовать во всех трёх местах, удалив новые дубли.

Test coverage

Новая логика покрыта неравномерно: есть unit-спеки cycle-guard и интеграционный тест конкурентных перемещений, а model-friendly-input.spec.ts покрывает основной путь форматтера. Без покрытия остаются:

  • [test-coverage] Покрыть unlocked else-ветку movePage (move-to-root / reorder под тем же родителем) — apps/server/src/core/page/services/page.service.ts:939-977
    Дифф разветвляет movePage на locked-путь (typeof parentPageId === 'string') и новую else-ветку, выполняющую this.pageRepo.updatePage(updateValues, dto.pageId) БЕЗ транзакции для move-to-root и same-parent reorder. Все movePage-спеки и интеграционный тест используют только parentPageId:'dest-parent' (locked-ветку); self-move отвергается до развилки. То есть самая частая бытовая операция — перемещение в корень/реордер — не прогоняется ни одним тестом; регрессия в условии развилки или в аргументах unlocked-update пройдёт весь suite. Fix: в describe «movePage cycle guard» добавить тест с dto.parentPageId === null (move-to-root), проверяющий, что updatePage вызван БЕЗ trx (mock.calls[0][2] undefined) и this.db.transaction не вызывался; опционально кейс same-parent reorder с теми же проверками.

  • [test-coverage] Покрыть ветку де-дупликации повторяющегося параметра в formatIssues — apps/server/src/core/ai-chat/tools/model-friendly-input.ts:456-468
    formatIssues реализует де-дуп повторяющихся имён параметров (if (seen.has(name)) continue;), но ни один тест не порождает два zod-issue на один path, так что ветка не исполняется. Косметика, низкий риск, но это заявленное новое поведение, ничем не зафиксированное. Fix: в model-friendly-input.spec.ts добавить форму, дающую два issue на один параметр (например refine + type error), и проверить, что имя параметра в сообщении не повторяется.

Architecture & design (forward-looking, non-blocking)

  • Шов валидации tool-input AI: дружелюбная ошибка привязана к одному потребителю общей spec — apps/server/src/core/ai-chat/tools/model-friendly-input.ts vs packages/mcp/src/tool-specs.ts (SharedToolSpec.buildShape), packages/mcp/src/index.ts
    Новый modelFriendlyInput() чисто консолидирует in-app путь (заменяет ~25 inline z.object(...) одним хелпером, который выводит JSON-схему draft-07 И прикрепляет дружелюбную ошибку: formatIssues + RETRY_HINT). Но источник схемы общий: и in-app сервисы, и standalone MCP-сервер строят input-схемы из одной SharedToolSpec.buildShape. PR добавляет дружелюбное поведение только на in-app ветке; MCP-сервер по-прежнему регистрирует сырой zod-shape, так что внешние MCP-клиенты на том же сценарии «потерял pageId в параллельном батче» (ради которого затевался #190) получат сырой zod-текст. Не дефект данного изменения — #190 был ограничен in-app чатом и там решён полностью; важно лишь если тот же сбой заденет внешнего MCP-клиента.
    • Вариант A — оставить как есть (effort: S). Pros: ноль работы; ровно соответствует scope #190; единственная пользовательская поверхность (in-app чат) исправлена; аудитория MCP — другие агенты, терпящие сырой zod-текст. Cons: два потребителя одной схемы имеют расходящийся UX ошибок; будущий запрос «friendly MCP errors» переизобретёт formatIssues/RETRY_HINT или потребует рефактора.
    • Вариант B — поднять форматтер ошибок в общий слой (packages/mcp) с opt-in для обоих потребителей (effort: M). Pros: formatIssues + RETRY_HINT становятся единым источником формулировок для in-app и standalone MCP; хелпер живёт там же, где схема; новые tool-ы наследуют единый UX. Cons: общая spec намеренно zod-/SDK-агностична (in-app — zod v4, MCP — v3; in-app-обёртка зависит от ai jsonSchema/Schema и z.toJSONSchema v4), вниз можно опустить только чистую часть formatIssues(ZodError)->string; аккуратное расщепление муторно и может быть избыточной абстракцией под единственный пока потребитель.
      Рекомендация ревьюера-архитектора: склонность к варианту A (поверхность #190 — in-app чат, она закрыта; чистый hoist нетривиален при отсутствии конкретного второго потребителя), плюс однострочная заметка у SharedToolSpec.buildShape, что friendly-error контракт пока живёт только в in-app обёртке.
## Code review — PR #200: дружелюбные ошибки tool-input в AI-чате (#190) + guard от циклов при конкурентных перемещениях страниц (#159) **Вердикт: Approve with comments.** Логика корректна и безопасна (security/regressions чисты, цикл при гонке re-parent действительно сериализуется через `FOR UPDATE` + re-check внутри транзакции). Блокирует только одно: по конвенции репозитория обе пользовательские правки должны попасть в `CHANGELOG.md [Unreleased]`, а он не тронут. Остальное — non-blocking. _Объём: дифф `develop`…`fix/issues-190-159` (merge-base `3ddc329b`), 8 файлов, +467/−79. Прогнаны параллельные аспектные ревьюеры (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) + judge-проход._ ### Must fix before merge - **[documentation] Добавить записи в `CHANGELOG.md [Unreleased]` для #190 и #159** — `CHANGELOG.md:11` Репозиторий ведёт Keep-a-Changelog с секцией `[Unreleased]`, и недавние PR (#163, #146/#147, #183) последовательно добавляли пункты по каждой пользовательской правке; ревью-коммиты прошлых PR трактовали отсутствие записи как дефект. PR везёт два user-facing изменения (исправление потери поддеревьев из-за цикла при конкурентном перемещении — #159; дружелюбные ошибки валидации tool-input в AI-чате с указанием параметра и retry-подсказкой вместо сырого zod-текста — #190), но `CHANGELOG.md` в диффе пустой. Fix: под `## [Unreleased]` добавить `### Fixed`-пункт для #159 (опаздывающие re-parent сериализуются `FOR UPDATE` с re-check цикла внутри транзакции) и `### Added`/`### Changed`-пункт для #190, в стиле существующих записей с ссылками на (#159) и (#190). ### Non-blocking - **[stability] Брать lock по каноническим UUID (`movedPage.id`, `parentPageId`), а не по сырому `dto.pageId`** — `apps/server/src/core/page/services/page.service.ts:943-944` Защита от дедлока держится на том, что оба встречных перемещения сортируют ОДНИ И ТЕ ЖЕ две строки-идентификатора и берут `FOR UPDATE` в одинаковом порядке. Сейчас `const lockIds = [dto.pageId, parentPageId].sort()`: `parentPageId` — уже разрешённый UUID (`parentPage.id`), а `dto.pageId` — сырое клиентское значение, и `findById` принимает также slugId (`MovePageDto.pageId` — неограниченный `@IsString`). Sidebar всегда шлёт UUID, так что на практике безопасно, но REST/agent-клиент, передавший slugId в двух одновременных встречных перемещениях (A: X под Y, B: Y под X), даст противоположный физический порядок блокировок и воспроизведёт AB-BA дедлок, который Postgres разрешит, отдав 500 вместо дружелюбного `BadRequestException`. Повреждения данных нет (цикл всё равно предотвращается), но контракт «не может задедлочиться» из комментария нарушается для slug-входа. Fix: `const lockIds = [movedPage.id, parentPageId].sort();` — `movedPage.id` это UUID, разрешённый контроллером, `parentPageId` уже UUID; `dto.pageId` для breadcrumb-check / `updatePage` оставить как есть (та же строка в транзакции). - **[simplification] Вынести дублированный stub `makeChain` (Proxy для chainable-trx) в один модульный хелпер** — `apps/server/src/core/page/services/page.service.spec.ts:46-56,327-337` PR добавляет две байт-в-байт копии Proxy-стаба `makeChain` поверх уже существующей в блоке `movePageToSpace()` на develop — три идентичные копии ~10-строчного хелпера в одном файле, каждую правку trx-стаба придётся делать в трёх местах (сами комментарии диффа отмечают, что «mirror the pattern»). Fix: завести один `makeChain()`/`makeTrxStub()` на уровне describe `PageService` (или модуля) и переиспользовать во всех трёх местах, удалив новые дубли. ### Test coverage Новая логика покрыта неравномерно: есть unit-спеки cycle-guard и интеграционный тест конкурентных перемещений, а `model-friendly-input.spec.ts` покрывает основной путь форматтера. Без покрытия остаются: - **[test-coverage] Покрыть unlocked else-ветку `movePage` (move-to-root / reorder под тем же родителем)** — `apps/server/src/core/page/services/page.service.ts:939-977` Дифф разветвляет `movePage` на locked-путь (`typeof parentPageId === 'string'`) и новую else-ветку, выполняющую `this.pageRepo.updatePage(updateValues, dto.pageId)` БЕЗ транзакции для move-to-root и same-parent reorder. Все movePage-спеки и интеграционный тест используют только `parentPageId:'dest-parent'` (locked-ветку); self-move отвергается до развилки. То есть самая частая бытовая операция — перемещение в корень/реордер — не прогоняется ни одним тестом; регрессия в условии развилки или в аргументах unlocked-update пройдёт весь suite. Fix: в describe «movePage cycle guard» добавить тест с `dto.parentPageId === null` (move-to-root), проверяющий, что `updatePage` вызван БЕЗ trx (`mock.calls[0][2]` undefined) и `this.db.transaction` не вызывался; опционально кейс same-parent reorder с теми же проверками. - **[test-coverage] Покрыть ветку де-дупликации повторяющегося параметра в `formatIssues`** — `apps/server/src/core/ai-chat/tools/model-friendly-input.ts:456-468` `formatIssues` реализует де-дуп повторяющихся имён параметров (`if (seen.has(name)) continue;`), но ни один тест не порождает два zod-issue на один path, так что ветка не исполняется. Косметика, низкий риск, но это заявленное новое поведение, ничем не зафиксированное. Fix: в `model-friendly-input.spec.ts` добавить форму, дающую два issue на один параметр (например refine + type error), и проверить, что имя параметра в сообщении не повторяется. ### Architecture & design (forward-looking, non-blocking) - **Шов валидации tool-input AI: дружелюбная ошибка привязана к одному потребителю общей spec** — `apps/server/src/core/ai-chat/tools/model-friendly-input.ts` vs `packages/mcp/src/tool-specs.ts` (`SharedToolSpec.buildShape`), `packages/mcp/src/index.ts` Новый `modelFriendlyInput()` чисто консолидирует in-app путь (заменяет ~25 inline `z.object(...)` одним хелпером, который выводит JSON-схему draft-07 И прикрепляет дружелюбную ошибку: `formatIssues` + `RETRY_HINT`). Но источник схемы общий: и in-app сервисы, и standalone MCP-сервер строят input-схемы из одной `SharedToolSpec.buildShape`. PR добавляет дружелюбное поведение только на in-app ветке; MCP-сервер по-прежнему регистрирует сырой zod-shape, так что внешние MCP-клиенты на том же сценарии «потерял pageId в параллельном батче» (ради которого затевался #190) получат сырой zod-текст. Не дефект данного изменения — #190 был ограничен in-app чатом и там решён полностью; важно лишь если тот же сбой заденет внешнего MCP-клиента. - **Вариант A — оставить как есть (effort: S).** Pros: ноль работы; ровно соответствует scope #190; единственная пользовательская поверхность (in-app чат) исправлена; аудитория MCP — другие агенты, терпящие сырой zod-текст. Cons: два потребителя одной схемы имеют расходящийся UX ошибок; будущий запрос «friendly MCP errors» переизобретёт `formatIssues`/`RETRY_HINT` или потребует рефактора. - **Вариант B — поднять форматтер ошибок в общий слой (`packages/mcp`) с opt-in для обоих потребителей (effort: M).** Pros: `formatIssues` + `RETRY_HINT` становятся единым источником формулировок для in-app и standalone MCP; хелпер живёт там же, где схема; новые tool-ы наследуют единый UX. Cons: общая spec намеренно zod-/SDK-агностична (in-app — zod v4, MCP — v3; in-app-обёртка зависит от `ai` jsonSchema/Schema и `z.toJSONSchema` v4), вниз можно опустить только чистую часть `formatIssues(ZodError)->string`; аккуратное расщепление муторно и может быть избыточной абстракцией под единственный пока потребитель. Рекомендация ревьюера-архитектора: склонность к варианту A (поверхность #190 — in-app чат, она закрыта; чистый hoist нетривиален при отсутствии конкретного второго потребителя), плюс однострочная заметка у `SharedToolSpec.buildShape`, что friendly-error контракт пока живёт только в in-app обёртке.
Ghost added 1 commit 2026-06-26 16:59:38 +03:00
fix(page,ai-chat): address PR #200 review — canonical lock UUIDs, changelog, tests 393f991c0f 
- page.service: lock concurrent re-parents by canonical row UUIDs
  (`movedPage.id`, not the raw client `dto.pageId` which may be a slugId) so
  two opposing moves passing slugIds can't sort into different FOR UPDATE
  orders and deadlock (#159).
- CHANGELOG: add [Unreleased] entries — Fixed #159 (serialized concurrent
  moves + in-tx cycle re-check), Added #190 (model-friendly tool-input errors).
- page.service.spec: hoist the duplicated `makeChain` trx-stub Proxy into one
  module-level helper (was 3 copies); add a move-to-root test covering the
  unlocked else-branch (no transaction opened, updatePage called without a trx).
- model-friendly-input.spec: cover the duplicate-path dedup branch in
  formatIssues — a single param with two zod issues is named only once.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ghost closed this pull request 2026-06-26 17:10:30 +03:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 idea
Idea / proposal for discussion
 invalid
This doesn't seem right
 needs-human
эскалация: нужно решение человека
 question
Further information is requested
 refactor
Code cleanup / refactoring
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
 security
Security / hardening issue
 status/blocked
ждёт зависимость blocked_by
 status/done
закрыто и проверено
 status/in-progress
в активной работе (мягкая заявка)
 status/ready
специфицировано, не заблокировано, ждёт исполнителя
 test
Test coverage / test infrastructure
 wontfix
This will not be worked on
No Label bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent_coder agent_reviewer vvzvlad
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#200
Delete Branch "fix/issues-190-159"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?