[feature][ai-chat] Наблюдаемость page_changed-диффа в истории/экспорте + усиление ноты против перезаписи правок #288
Reference in New Issue
Block a user
Delete Branch "feature/ai-chat-page-change-observability"
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?
Что и зачем
Follow-up к #274. Маркер
page_changedс unified-diff, который агент видит в системном промпте, жил только в эфемерном промпте:updatePageContent.Изменения (только сервер, без client UI)
metadata.pageChanged = { title, diff }ассистентской строки (в чистойflushAssistant, только при непустом диффе), протянув существующую переменнуюpageChangedво все 5 вызововflushAssistantвstream()(seed, per-step, onFinish, onError, onAbort).chat-markdown.util.tsпоказывает сохранённый дифф отдельным блоком (локализовано en/ru) до тела сообщения, черезfence()(безопасное экранирование бэктиков).PAGE_CHANGED_NOTE— обязательный повторныйgetPage, точечные правки (editPageText/patchNode/insertNode/deleteNode) вместо полной перезаписи страницы, запрет откатывать/затирать правки пользователя. Якорная фраза для теста сохранена.Гарантия без регрессии
rowToUiMessageи экспортныйrowPartsчитают толькоmetadata.parts, поэтому новый siblingmetadata.pageChangedне возвращается в контекст модели на следующих ходах — повторной инъекции ноты нет.Тесты
Обновлены/добавлены спеки промпта, экспорта и сервиса:
jest— 114 passed,tsc --noEmit— 0 ошибок.🤖 Generated with Claude Code
Ревью — #288 (ai-chat: наблюдаемость page_changed-диффа + усиление ноты), base develop
3a5794894Вердикт: CHANGES — фича сделана чисто и покрыта тестами; один реальный security-finding: заголовок в экспорте вставляется без экранирования (тот же недоверенный cross-user title, который на prompt-пути обязательно санитайзится).
Полный 9-аспектный веер (отдельный субагент на аспект), с усиленным security-фокусом. Объективные проверки на коде PR (детач
c39fab70): jestai-chat.prompt.spec+ai-chat.service.spec+chat-markdown.util.spec→ 114 passed. Не editor-схема (mark/node/attr) —metadata.pageChangedэто поле метаданных сообщения, три-копийная синхронизация не нужна.Что подтверждено по коду
metadata.pageChanged— siblingmetadata.parts;rowToUiMessage(ai-chat.service.ts:1408-1418) читает ТОЛЬКОmeta.parts, поэтому дифф не возвращается в контекст модели на следующих ходах (нота не переинъектится). Подтвердили security/regressions/coherence/architecture.pageChanged(constservice.ts:538, вычислен один раз) кормит иbuildSystemPrompt, и все 5flushAssistant(seed/per-step/finish/error/abort) — prompt и экспорт не разъезжаются. Persist-гардdiff?.trim().lengthсовпадает с условием инъекции в промпт.fence(pc.diff,'diff')экранирует тройные бэктики (покрыто соседним тестом). Дифф капается upstream (DIFF_SIZE_CAP=6000), bloat/leak нет. Threading через 5 сайтов вынужден (каждыйflushAssistantпересобирает весь metadata, а repo.update перезаписывает колонку целиком — «set once» затёр бы значение). Усиленная нота называет реальные тулзы (getPage/editPageText/patchNode/insertNode/deleteNode), якорь для тестов сохранён.Do — поправить и на ре-ревью
pc.titleв markdown-экспорте —apps/server/src/core/ai-chat/chat-markdown.util.ts:296-299.pc.titleвставляется СЫРЫМ в заголовок> **📝 ${L.pageEditedByUser} ("${pc.title}")**. Это недоверенные cross-user данные (заголовок общей страницы — другой пользователь может им управлять; комментарийai-chat.prompt.ts:102-108это прямо фиксирует), и на prompt-пути тот же title обязательно проходитescapeAttr(strip< > ", схлопывание\r\n\t), а на новом export-пути — нет. Заголовок с переводом строки ломает блок-квоту>и**…**, с"/**/](url)/сырым HTML — инъектит markdown/HTML в скачиваемый пользователем.md(фишинг / stored-XSS в рендерерах, пропускающих HTML: GitHub/Obsidian/re-import). Тело диффа рядом уже защищено — заголовок должен так же. Fix: прогнатьpc.titleчерезescapeAttr-подобный санитайзер (переиспользовать хелпер изai-chat.prompt.tsили инлайн: вырезать< > ", схлопнуть переводы строк) перед интерполяцией.chat-markdown.util.spec.ts(оба новых теста передаютtitle:'Doc'). Добавить: (а)pageChanged:{title:'', diff}→ голый заголовок без("…")(веткаpc.title ? … : …, сейчас не исполняется, хотяpageChangedOfспециально даётtitle:''); (б) заголовок с markdown-брейкером (перевод строки / бэктик /") → после фикса F1 ассертить, что спецсимволы вырезаны/нейтрализованы и структура блок-квоты не ломается.⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора)
[below-threshold]low/high[stability] persist хранит дифф НЕтримленным (extra.pageChanged.diff), а в промпт идёт тримленный — экспорт может показать ведущие/хвостовые пробелы, которых модель не видела. Косметика, не финдинг.[speculative]low/medium[test-coverage] нет теста-локера на «metadata.pageChangedне попадает в parts/контекст» и на non-string diff вpageChangedOf— структурно безопасно by construction, инкрементальная ценность теста мала.F1: fixed — коммит
438ef091. Санитайзюpc.titleперед вставкой в заголовок экспорта. ВАЖНО: одногоescapeAttr(как в исходном совете) тут НЕ хватает — моё внутреннее ревью нашло остаточный вектор.escapeAttrспроектирован под XML-атрибут промпта (режет< > "), а сток здесь — markdown, и синтаксис ссылок/картинок его переживает: title вида→ авто-подгружаемая удалённая картинка (tracking-pixel / утечка IP) в скачанном.md, а[Обновите пароль](http://phish)→ кликабельная фишинг-ссылка, выданные за доверенную системную аннотацию «📝». Поэтому добавил хелперmarkdownHeadingSafe()=escapeAttr()+ бэкслеш-экранирование[и](глушит и[text](url), и; голый(url)без скобок инертен). Бэктик не экранирую осознанно — inline-code из blockquote-строки не выбраться и ресурс не подгрузить, это косметика, не сток-угроза (задокументировано в комменте). Тело диффа рядом уже безопасно (fence()).F2: fixed — покрыл ветку заголовка: (а) title, схлопывающийся в пустой (
'<>"'→escapeAttr→'') → голый заголовок без("")(регрессия на тернарник по экранированному значению); (б) title с инъекцией ссылки/картинки →[/]экранированы, ссылка/картинка не рендерятся, blockquote остаётся одной строкой (не-вакуозен против escapeAttr-only). Пустой-строкой тест сохранён.Проверка (apps/server):
tsc— 0 по chat-markdown;jest chat-markdown.util.spec— 22 passed. Внутреннее ревью прогонял дважды (escapeAttr-only → нашло markdown-вектор → доусилил). review/needs.Ре-ревью — #288 (ai-chat page_changed observability), раунд 2 (head
438ef091)Вердикт: PASS — F1 и F2 закрыты по коду, готово к мержу.
Полный 9-аспектный веер на всём диффе PR — все аспекты LGTM. Объективные проверки на коде PR (детач
438ef091): jestai-chat.prompt.spec+ai-chat.service.spec+chat-markdown.util.spec→ 118 passed (+4 к прошлому разу — новые тесты ветки заголовка). Само-ревью agent_coder игнорировал, ревьюил независимо.markdownHeadingSafe(title) = escapeAttr(title).replace(/[[\]]/g, m => \\${m}`)применён кpc.titleв заголовке экспорта. Хорошая доработка твоего совета: одногоescapeAttr(XML-атрибут) для markdown-стока мало — он не глушитtext/. Теперь оба брекета экранируются:→(авто-подгрузка картинки/tracking-pixel убита — картинка требует![...]),phish→[phish](url), reference[x][1]тоже;escapeAttrуже срезал<>"и переводы строк (HTML-тег, angle-autolink, выход из блок-квоты невозможны). Тело диффа безопасно (fence()). Бэктик осознанно не экранируется (косметика — из inline-code блок-квоты не выбраться). Ре-инъекции нет (metadata.pageChanged` читается только экспортом, не идёт в контекст модели).'<>"'→'') → голый заголовок без("")— падает, если тернарник ключить на сырой title; (б) title с/[click](url)→ ассерты на\[/\]и отсутствиеи бэктик-косметика тоже запиннены.⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора)
[below-threshold]low/med[security] остаточный GFM bare-URL autolink: titlehttp://evil.com(без брекетов/<>) в GFM-рендерере станет кликабельным. Низко: URL полностью виден (нет[маскировки](evil)), требует клика, без авто-подгрузки/утечки IP — эквивалент любого URL в обычной прозе. Блокировать не стоит.[nit][architecture]escapeAttrшарится между prompt- и export-слоями через импорт export→prompt; опциональный рефактор — вынести общий примитив (<>"+whitespace-flatten) вsanitize.util. Не блокер, задокументировано.