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
 epic
Large multi-phase effort spanning many changes
 feature
New functionality request
 idea
Idea / proposal for discussion
 needs-human
эскалация: нужно решение человека
 review/approved
в последнем ревью нет открытых blocking-находок
 review/changes-requested
последнее ревью оставило открытые blocking-находки
 review/needs
head не ревьюился (head != reviewed_head)
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?