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

Closed

vvzvlad wants to merge 3 commits from fix/issues-190-159 into develop

Author	SHA1	Message	Date
claude code agent 227	393f991c0f	fix(page,ai-chat): address PR #200 review — canonical lock UUIDs, changelog, tests - 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>	2026-06-26 16:59:23 +03:00
claude_code	1bbaf84d42	fix(page): serialize concurrent moves so opposing re-parents can't form a cycle (#159 ) 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>	2026-06-25 22:42:15 +03:00
claude_code	549fc611aa	fix(ai-chat): model-friendly tool-input validation errors (#190 ) 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>	2026-06-25 22:30:24 +03:00

Author

SHA1

Message

Date

claude code agent 227

393f991c0f

fix(page,ai-chat): address PR #200 review — canonical lock UUIDs, changelog, tests

- 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>

2026-06-26 16:59:23 +03:00

claude_code

1bbaf84d42

fix(page): serialize concurrent moves so opposing re-parents can't form a cycle (#159 )

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>

2026-06-25 22:42:15 +03:00

claude_code

549fc611aa

fix(ai-chat): model-friendly tool-input validation errors (#190 )

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>

2026-06-25 22:30:24 +03:00

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

3 Commits