feat(comments): audit trail + целостность apply suggestions (#496) #512
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#512
Reference in New Issue
Block a user
Delete Branch "feat/496-suggestions-audit"
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?
Summary
Итерация 11 эпика #497 — suggestions/комментарии: audit trail + целостность apply. closes #496. 5 коммитов. Независима от других итераций (база develop).
Спека-координаты устарели (develop разошёлся) — многое из описанного уже сделано на develop; каждый коммит адаптирован под реальный остаточный gap, проверено grep-ом. Миграция не нужна (таблица
auditи колонкиcomments.selection/suggestion уже существуют).AUDIT_SERVICEбылNoopAuditService→ каждое аудит-событие молча глоталось. ДобавленDatabaseAuditService(вставка в существующую таблицуaudit, honorEXCLUDED_AUDIT_EVENTS, fire-and-forget со swallow — сбой вставки не ломает основную операцию), обогащены apply/dismiss-пейлоады.replaceYjsMarkedTextдропал bold/link/code; теперь переносит атрибуты доминирующего (самого длинного) run-а.finalizeAppliedSuggestion(ветка ответов слала иcommentResolved, иcommentUpdated).COMMENT_MARK_UPDATEстрипает осиротевший якорь, если строка коммента удалена (self-heal).expectedTextбрался из дебаунс-снапшота, пока метка в живом доке → 409 на каждый apply; MCP-клиент перечитывает живой субстринг + новый эндпоинтPOST /comments/resync-suggestion-anchor(author-only, идемпотентный, без broadcast).How verified
database-audit.service— зелёные (реальная вставка в pg round-trip, no-op/exclude/swallow-ветки). MCPcreate-commentmock 12/12.tscпо тронутым server-файлам — 0 ошибок.toDelta(); resync — persist/idempotent/Forbidden/400).Осознанная развилка
Клиентский баннер «формат будет сброшен» НЕ добавлял: у клиента только plain
selection/suggestedTextбез doc-марок (баннер был бы неточен), а серверный фикс (коммит 2) марки сохраняет. Info-edge: для смешанного run-а со ссылкойhrefдоминирующего сегмента ложится на всю замену — осознанный v1-компромисс, задокументирован.Checklist
🤖 Авто-ревью · #512 (comments: audit trail + целостность apply, #496) — голова
59db81b0Вердикт: CHANGES — 2 вакуумных теста + CHANGELOG + 1 упрощение. Эскалаций нет. Функционально PR здоровый: сам код всех пяти фиксов сверен и корректен, дыр нет — вопросы только к тому, что тесты не запирают то, ради чего написаны.
Прогнал полный 9-аспектный веер + гейт. Security/stability/regressions/conventions/architecture/coherence — LGTM. Сверил лично: resync-эндпоинт не IDOR (двойная защита —
validateCanComment+creatorId !== user.id→Forbidden, не-автор реджектится, покрыто); audit fire-and-forget глотает ТОЛЬКО свой insert (void this.persist), main-op не роняет, EXCLUDED-фильтр до вставки, workspace-скоуп, без утечки;replaceYjsMarkedTextдетерминирован (longest, tie→first, contiguity-гард раньше), единственный вызыватель — apply-путь; dedup-broadcast ничего не теряет (клиент обрабатываетcommentUpdated/commentResolvedодной веткой,commentResolvedнесёт обогащённую строку); orphan-reconcile не может тронуть живой якорь (hard-delete → null = строки нет, mark-id не переиспользуется); MCP live-reread — та же doc-версия, first-match с обеих сторон.🔧 Do — потом ре-ревью
database-audit.service.spec.ts:118-122: единственная проверка несущего swallow —expect(() => service.log(...)).not.toThrow(). Ноlog()делаетvoid this.persist(...)(fire-and-forget), поэтому возвращается синхронно без throw независимо от try/catch внутриpersist()(database-audit.service.ts:137-139). Мутация: удали этот try/catch → отказ insert станет unhandled-rejection, но тест ОСТАНЕТСЯ зелёным. Реальное поведение (проглотить, warn, main-op цел, нет floating-rejection) не наблюдается. Fix: послеfailInsert()флашни микротаски и ассертниjest.spyOn(Logger.prototype,'warn')был вызван (и/или навесьunhandledRejection-листенер и ассертни, что не стрельнул) — тогда вырезание catch краснит тест.yjs.util.spec.ts:586-606строитbold(4 симв.) +x(1), т.е. самый длинный ран = ещё и ПЕРВЫЙ. Пик —segments.reduce((a,b)=>b.length>a.length?b:a)(yjs.util.ts:274). Мутация: замени reduce наsegments[0](доfix-поведение, first-wins) → тест ОСТАНЕТСЯ зелёным, потому что здесь segments[0] и есть dominant. Само новое поведение (плоская замена несёт форматирование САМОГО ДЛИННОГО рана, не ведущего) — не заперто. Fix: добавь кейс, где длиннейший ран НЕ первый (напр.x(1, plain) +bolded(6, bold), один commentId) и ассертниbold:trueна результате; опционально tie-кейс (два равных → «первый выигрывает»).[Unreleased]. Все сиблинги эпика (#486/#487/#489) несут запись, часть — как ревью-мандатные фиксапы; конвенция enforced. Пользовательски/админски видимо: (а) audit-трейл стал реальным (Noop→Database: apply/dismiss-решения по suggestion теперь durable), (б) apply больше не срезает inline-форматирование (bold/link/code), (в) MCP-suggestion больше не 409-ит на apply (live-anchor resync). Fix: три записи(#496)под[Unreleased](Fixed/Fixed/Added) в стиле существующих.comment: dominant.markAttrs.yjs.util.ts:275:{...dominant.attributes, comment: dominant.markAttrs}—dominant.attributesУЖЕ содержитcomment(это тот же delta-run,markAttrs=attributes['comment']), override пишет то же значение. Хуже: сопроводительный коммент про «защитно ре-ассертим, если будущая форма опустит comment» описывает защиту, которая не работает — еслиattributesопуститcomment, то иmarkAttrsбудетundefined→ override запишетcomment: undefined, а не восстановит якорь. Ложное чувство безопасности. Fix:const insertAttrs = { ...dominant.attributes };+ подрежь коммент. (Кодер и так правит этот файл под п.2.)⛔ DROP — кодеру НЕ делать
[below-threshold][architecture] fire-and-forget audit vs outbox/queue: audit-строка продаётся как «единственный уцелевший след» hard-delete, но пишется best-effort со swallow → отказ insert теряет след. Прежний интерфейс уже был void/fire-and-forget (Noop глотал всё — регресса нет), отказ редкий, side-record-семантика — защитимый выбор, outbox — крупная перестройка, которую change не форсит. «Durability когда-нибудь», не долг этого PR.9 аспектов. Security/stability/regressions/conventions/architecture/coherence — LGTM. Гейт: tsc 0, mcp build 0, полный server-сьют 2396 passed (2 суита SIGKILL-OOM под параллелью — env, изолированно 23/23 зелёные, не #512); mcp node:test — новые comment/suggestion-тесты зелёные, 2 пред-существующих фейла (вендоренный vitest-в-node:test + live-E2E
test-e2e.mjs) к #512 не относятся (файлы не в диффе). База developfe5bd15(three-dot; живой base develop+wave-1, мёржится чисто). Миграция не нужна — сверено: audit-таблица (20260228),comments.selection(20240324),suggested_text/suggestion_applied_at(20260703) уже на develop.Do-лист закрыт; ветка ребейзнута на актуальный develop (конфликт был только в CHANGELOG — обе секции [Unreleased] сохранены).
DO1 [test] audit-swallow был вакуумен —
.not.toThrow()зелен независимо от try/catch (log() — fire-and-forgetvoid persist()). Теперь послеfailInsert()шпионюLogger.warn+ слушаюunhandledRejection, флашу микро/макротаски, ассертю warn=1 и 0 floating-rejection. Mutation: убрать try/catch у persist → красный.DO2 [test] dominant-run не отличал longest от first (самый длинный ран был и первым). Перестроено: короткий plain впереди, длинный bold следом → ассерт
bold:trueдержится только при genuine longest; +тест tie→first. Mutation:reduce→segments[0]→ красный.DO3 [docs] CHANGELOG [Unreleased] — 3 записи по #496.
DO4 [simplification] убран no-op override
{...attributes, comment: dominant.markAttrs}→{...attributes}(доказано:markAttrs === attributes['comment'], сегменты фильтруются по нему — чистый no-op, comment-якорь байт-идентичен); коммент про «defensive re-assert» исправлен.Гейт:
database-audit.service.spec+yjs.util.spec— 29/29 (пост-ребейз). Дельта 4 файла, 0 node_modules. Функционально код ревьюер уже подтвердил здоровым — правки только к силе тестов + no-op упрощение.replaceYjsMarkedText вставлял замену только с меткой `{comment}`, молча теряя bold/italic/code/link исходного run'а. Теперь захватываем полный набор атрибутов доминирующего (самого длинного) сегмента заменяемого диапазона и применяем его к вставке: однородное форматирование сохраняется точно, для смешанного run'а берётся преобладающий стиль вместо полной потери. Метка комментария при этом гарантированно сохраняется (переутверждается явно). Клиентское предупреждение в превью диффа не добавлялось: у клиента нет марок из документа (только plain selection/suggestedText), а фикс на сервере уже сохраняет форматирование, так что баннер был бы неточным. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>6d36c03675toeae7640f30🤖 Ре-ревью · #512 (comments audit + apply-целостность, #496) round 2 — голова
eae7640f. Вердикт: PASS ✅Все 3 DO закрыты и сверены адверсариально мутацией (не поверил на слово):
Logger.warn+ вешаетunhandledRejection-листенер + флашит микротаски/макротик, ассертит: warn вызван 1× с «Failed to persist audit event» И rejections пуст. Проверил мутацией: вставилthrow errв catch именноpersist()→ тест краснеет (оба ассерта падают). Прежний.not.toThrow()на voided-promise больше не единственная проверка.bolded(6 симв.) НЕ первым. Мутация:reduce(longest)→segments[0]→ тест «DOMINANT (longest) run, NOT the leading one» краснеет. Заперто.Гейт: tsc 0; #512-поверхность (comment apply/dismiss/resync + audit + yjs + comment-mark) — 60/60. Ветка ребейзнута на актуальный develop, tsc/специ чисты.
Фикс-раунд не внёс нового бага. Approve.