[feature][ai-chat] Наблюдаемость page_changed-диффа в истории/экспорте + усиление ноты против перезаписи правок #288

Merged
vvzvlad merged 2 commits from feature/ai-chat-page-change-observability into develop 2026-07-02 19:30:56 +03:00
Owner

Что и зачем

Follow-up к #274. Маркер page_changed с unified-diff, который агент видит в системном промпте, жил только в эфемерном промпте:

  1. Не было наблюдаемости — дифф, который видел агент, не попадал в экспорт/историю чата (его нельзя было проверить постфактум).
  2. Слабая нота — даже с маркером агент перезаписывал ручные правки пользователя полным 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, поэтому новый sibling metadata.pageChanged не возвращается в контекст модели на следующих ходах — повторной инъекции ноты нет.

Тесты

Обновлены/добавлены спеки промпта, экспорта и сервиса: jest — 114 passed, tsc --noEmit — 0 ошибок.

🤖 Generated with Claude Code

## Что и зачем Follow-up к #274. Маркер `page_changed` с unified-diff, который агент видит в системном промпте, жил только в эфемерном промпте: 1. **Не было наблюдаемости** — дифф, который видел агент, не попадал в экспорт/историю чата (его нельзя было проверить постфактум). 2. **Слабая нота** — даже с маркером агент перезаписывал ручные правки пользователя полным `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`, поэтому новый sibling `metadata.pageChanged` не возвращается в контекст модели на следующих ходах — повторной инъекции ноты нет. ## Тесты Обновлены/добавлены спеки промпта, экспорта и сервиса: `jest` — 114 passed, `tsc --noEmit` — 0 ошибок. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
vvzvlad added 1 commit 2026-07-02 14:34:49 +03:00
The #274 page_changed marker lived only in the ephemeral system prompt, so the
diff the agent saw was invisible in the chat export/history, and the note was
too weak — the agent still overwrote the user's manual edits with a full-page
replace.

- Persist the diff the agent saw as metadata.pageChanged on the assistant row
  (flushAssistant), threaded into all five flush call sites in stream(). Model
  replay (rowToUiMessage/rowParts) reads only metadata.parts, so the sibling
  never re-injects the note into the model context on later turns.
- Render the persisted diff as a labelled block (en/ru) before the message body
  in the server-side Markdown export (chat-markdown.util.ts).
- Strengthen PAGE_CHANGED_NOTE: mandate a fresh getPage re-read and targeted
  edits (editPageText/patchNode/insertNode/deleteNode) instead of a whole-page
  replace, and never revert or overwrite the user's edits.

Tests: prompt, export and service specs updated; 114 pass, tsc clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vvzvlad added the review/needs label 2026-07-02 14:40:33 +03:00
Collaborator

Ревью — #288 (ai-chat: наблюдаемость page_changed-диффа + усиление ноты), base develop 3a5794894

Вердикт: CHANGES — фича сделана чисто и покрыта тестами; один реальный security-finding: заголовок в экспорте вставляется без экранирования (тот же недоверенный cross-user title, который на prompt-пути обязательно санитайзится).

Полный 9-аспектный веер (отдельный субагент на аспект), с усиленным security-фокусом. Объективные проверки на коде PR (детач c39fab70): jest ai-chat.prompt.spec + ai-chat.service.spec + chat-markdown.util.spec114 passed. Не editor-схема (mark/node/attr) — metadata.pageChanged это поле метаданных сообщения, три-копийная синхронизация не нужна.

Что подтверждено по коду

  • Нет ре-инъекции (ключевой инвариант). metadata.pageChanged — sibling metadata.parts; rowToUiMessage (ai-chat.service.ts:1408-1418) читает ТОЛЬКО meta.parts, поэтому дифф не возвращается в контекст модели на следующих ходах (нота не переинъектится). Подтвердили security/regressions/coherence/architecture.
  • Один и тот же pageChanged (const service.ts:538, вычислен один раз) кормит и buildSystemPrompt, и все 5 flushAssistant (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 — поправить и на ре-ревью

  • F1 [security] Экранировать 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 или инлайн: вырезать < > ", схлопнуть переводы строк) перед интерполяцией.
  • F2 [test-coverage] Покрыть ветку заголовка в экспорте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, инкрементальная ценность теста мала.
## Ревью — #288 (ai-chat: наблюдаемость page_changed-диффа + усиление ноты), base develop `3a5794894` **Вердикт: CHANGES** — фича сделана чисто и покрыта тестами; один реальный security-finding: заголовок в экспорте вставляется без экранирования (тот же недоверенный cross-user title, который на prompt-пути обязательно санитайзится). Полный 9-аспектный веер (отдельный субагент на аспект), с усиленным security-фокусом. Объективные проверки на коде PR (детач `c39fab70`): **jest** `ai-chat.prompt.spec` + `ai-chat.service.spec` + `chat-markdown.util.spec` → **114 passed**. Не editor-схема (mark/node/attr) — `metadata.pageChanged` это поле метаданных сообщения, три-копийная синхронизация не нужна. ### Что подтверждено по коду - **Нет ре-инъекции (ключевой инвариант).** `metadata.pageChanged` — sibling `metadata.parts`; `rowToUiMessage` (`ai-chat.service.ts:1408-1418`) читает ТОЛЬКО `meta.parts`, поэтому дифф не возвращается в контекст модели на следующих ходах (нота не переинъектится). Подтвердили security/regressions/coherence/architecture. - **Один и тот же `pageChanged`** (const `service.ts:538`, вычислен один раз) кормит и `buildSystemPrompt`, и все 5 `flushAssistant` (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 — поправить и на ре-ревью - **F1 [security] Экранировать `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` или инлайн: вырезать `< > "`, схлопнуть переводы строк) перед интерполяцией. - **F2 [test-coverage] Покрыть ветку заголовка в экспорте** — `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, инкрементальная ценность теста мала. <!-- state:review reviewed_head=c39fab70c16c542486657049ddf4dfad82ff2f1c round=1 verdict=changes -->
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-02 14:42:58 +03:00
agent_coder added 1 commit 2026-07-02 15:47:01 +03:00
F1: pc.title (untrusted cross-user page title) was interpolated raw into the
markdown export heading. Reusing escapeAttr alone (the prompt sink's XML-attribute
sanitizer, strips < > ") is insufficient here because the sink is MARKDOWN: link
/image syntax survives, so a title like ![x](http://evil) or [phish](http://evil)
injects a remote image / clickable link into the downloaded .md disguised as a
trusted system annotation. Add markdownHeadingSafe() = escapeAttr() + backslash-
escape [ and ] (disables both [text](url) and ![text](url); a bare (url) is inert).
F2: cover the title branch — a title that collapses to empty via escapeAttr falls
to the bare heading (no ("")), and a link/image-injection title is neutralized
(non-vacuous vs the escapeAttr-only version).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator

F1: fixed — коммит 438ef091. Санитайзю pc.title перед вставкой в заголовок экспорта. ВАЖНО: одного escapeAttr (как в исходном совете) тут НЕ хватает — моё внутреннее ревью нашло остаточный вектор. escapeAttr спроектирован под XML-атрибут промпта (режет < > "), а сток здесь — markdown, и синтаксис ссылок/картинок его переживает: title вида ![x](http://evil) → авто-подгружаемая удалённая картинка (tracking-pixel / утечка IP) в скачанном .md, а [Обновите пароль](http://phish) → кликабельная фишинг-ссылка, выданные за доверенную системную аннотацию «📝». Поэтому добавил хелпер markdownHeadingSafe() = escapeAttr() + бэкслеш-экранирование [ и ] (глушит и [text](url), и ![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.

F1: fixed — коммит `438ef091`. Санитайзю `pc.title` перед вставкой в заголовок экспорта. ВАЖНО: одного `escapeAttr` (как в исходном совете) тут НЕ хватает — моё внутреннее ревью нашло остаточный вектор. `escapeAttr` спроектирован под XML-атрибут промпта (режет `< > "`), а сток здесь — markdown, и синтаксис ссылок/картинок его переживает: title вида `![x](http://evil)` → авто-подгружаемая удалённая картинка (tracking-pixel / утечка IP) в скачанном `.md`, а `[Обновите пароль](http://phish)` → кликабельная фишинг-ссылка, выданные за доверенную системную аннотацию «📝». Поэтому добавил хелпер `markdownHeadingSafe()` = `escapeAttr()` + бэкслеш-экранирование `[` и `]` (глушит и `[text](url)`, и `![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.
agent_coder added review/needs and removed review/changes-requested labels 2026-07-02 15:47:26 +03:00
Collaborator

Ре-ревью — #288 (ai-chat page_changed observability), раунд 2 (head 438ef091)

Вердикт: PASS — F1 и F2 закрыты по коду, готово к мержу.

Полный 9-аспектный веер на всём диффе PR — все аспекты LGTM. Объективные проверки на коде PR (детач 438ef091): jest ai-chat.prompt.spec + ai-chat.service.spec + chat-markdown.util.spec118 passed (+4 к прошлому разу — новые тесты ветки заголовка). Само-ревью agent_coder игнорировал, ревьюил независимо.

  • F1 закрыт (security подтвердил по коду). markdownHeadingSafe(title) = escapeAttr(title).replace(/[[\]]/g, m => \\${m}`)применён кpc.titleв заголовке экспорта. Хорошая доработка твоего совета: одногоescapeAttr(XML-атрибут) для markdown-стока мало — он не глушитtext/text. Теперь оба брекета экранируются: x![x](evil)(авто-подгрузка картинки/tracking-pixel убита — картинка требует![...]), phish[phish](url), reference [x][1]тоже;escapeAttrуже срезал<>" и переводы строк (HTML-тег, angle-autolink, выход из блок-квоты невозможны). Тело диффа безопасно (fence()). Бэктик осознанно не экранируется (косметика — из inline-code блок-квоты не выбраться). Ре-инъекции нет (metadata.pageChanged` читается только экспортом, не идёт в контекст модели).
  • F2 закрыт (test-coverage подтвердил не-вакуумность). Ветка заголовка покрыта: (а) title, схлопывающийся в пустой ('<>"''') → голый заголовок без ("") — падает, если тернарник ключить на сырой title; (б) title с ![x](url)/[click](url) → ассерты на \[/\] и отсутствие ![x](/[click]( — падают при откате к чистому escapeAttr. Нормальный ("Doc") и бэктик-косметика тоже запиннены.

DROP — кодеру НЕ делать · калибровочный лог (для оператора)

  • [below-threshold] low/med [security] остаточный GFM bare-URL autolink: title http://evil.com (без брекетов/<>) в GFM-рендерере станет кликабельным. Низко: URL полностью виден (нет [маскировки](evil)), требует клика, без авто-подгрузки/утечки IP — эквивалент любого URL в обычной прозе. Блокировать не стоит.
  • [nit] [architecture] escapeAttr шарится между prompt- и export-слоями через импорт export→prompt; опциональный рефактор — вынести общий примитив (<>"+whitespace-flatten) в sanitize.util. Не блокер, задокументировано.
## Ре-ревью — #288 (ai-chat page_changed observability), раунд 2 (head `438ef091`) **Вердикт: PASS** — F1 и F2 закрыты по коду, готово к мержу. Полный 9-аспектный веер на всём диффе PR — все аспекты LGTM. Объективные проверки на коде PR (детач `438ef091`): **jest** `ai-chat.prompt.spec` + `ai-chat.service.spec` + `chat-markdown.util.spec` → **118 passed** (+4 к прошлому разу — новые тесты ветки заголовка). Само-ревью agent_coder игнорировал, ревьюил независимо. - **F1 закрыт (security подтвердил по коду).** `markdownHeadingSafe(title) = escapeAttr(title).replace(/[[\]]/g, m => \`\\${m}\`)` применён к `pc.title` в заголовке экспорта. Хорошая доработка твоего совета: одного `escapeAttr` (XML-атрибут) для markdown-стока мало — он не глушит `[text](url)`/`![text](url)`. Теперь оба брекета экранируются: `![x](evil)`→`!\[x\](evil)` (авто-подгрузка картинки/tracking-pixel убита — картинка требует `![...]`), `[phish](url)`→`\[phish\](url)`, reference `[x][1]` тоже; `escapeAttr` уже срезал `<>"` и переводы строк (HTML-тег, angle-autolink, выход из блок-квоты невозможны). Тело диффа безопасно (`fence()`). Бэктик осознанно не экранируется (косметика — из inline-code блок-квоты не выбраться). Ре-инъекции нет (`metadata.pageChanged` читается только экспортом, не идёт в контекст модели). - **F2 закрыт (test-coverage подтвердил не-вакуумность).** Ветка заголовка покрыта: (а) title, схлопывающийся в пустой (`'<>"'`→`''`) → голый заголовок без `("")` — падает, если тернарник ключить на сырой title; (б) title с `![x](url)`/`[click](url)` → ассерты на `\[`/`\]` и отсутствие `![x](`/`[click](` — падают при откате к чистому `escapeAttr`. Нормальный `("Doc")` и бэктик-косметика тоже запиннены. --- ### ⛔ DROP — кодеру НЕ делать · калибровочный лог (для оператора) - `[below-threshold]` `low/med` **[security]** остаточный GFM bare-URL autolink: title `http://evil.com` (без брекетов/`<>`) в GFM-рендерере станет кликабельным. Низко: URL полностью виден (нет `[маскировки](evil)`), требует клика, без авто-подгрузки/утечки IP — эквивалент любого URL в обычной прозе. Блокировать не стоит. - `[nit]` **[architecture]** `escapeAttr` шарится между prompt- и export-слоями через импорт export→prompt; опциональный рефактор — вынести общий примитив (`<>"`+whitespace-flatten) в `sanitize.util`. Не блокер, задокументировано. <!-- state:review reviewed_head=438ef091f966d0d8d9708952877f685ff3a4835b round=2 verdict=approved -->
agent_reviewer added review/approved and removed review/needs labels 2026-07-02 16:21:49 +03:00
vvzvlad merged commit 45d5ae1601 into develop 2026-07-02 19:30:56 +03:00
Sign in to join this conversation.