fix(mcp): drawio — авто-стрип XML-комментариев (гейт на well-formedness) вместо lint-ошибки (#505) #545
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#545
Reference in New Issue
Block a user
Delete Branch "fix/505-drawio-strip-comments"
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
Живой in-app агент постоянно спотыкается о жёсткую lint-ошибку
[no-comments](модель органически вставляет<!-- -->в mxGraph-XML — природа LLM, промптом не лечится) и делает полный ре-генерейт диаграммы — лишний tool-вызов с полным XML почти на каждой первой генерации. Фикс: авто-стрип комментариев + warning вместо ошибки. closes #505.Что сделано
prepareModel()(drawio-xml.ts), которым идут все три инструмента (drawioCreate/drawioUpdate/drawioEditCells— один шов, без дублирования), ПЕРЕДlintModel:rawModel.replace(/<!--[\s\S]*?-->/g, "").stripped N XML comment(s)вwarnings[]только при N>0 — модель видит, но НЕ ретраит.no-commentsоставлено как defense-in-depth (нормальным путём после стрипа недостижимо, но остаётся backstop).Ключевой момент безопасности (доводка после ревью)
Стрип гейтится well-formedness: сначала
parseXml(rawModel)(тот же jsdom-парсер, что уlintModel/wrapCellFragment), комментарии режутся ТОЛЬКО приparseError === null. На malformed-входе (сырой неэкранированный<в значении атрибута, напр.value="a <!-- secret --> b") стрип пропускается, вход без изменений уходит вlintModel→ отклоняетсяvalue-escaping/well-formed-xml, автор видит реальную ошибку. Без гейта такой вход тихо усекался бы в"a b"и принимался — это была бы регрессия гарантий well-formed-xml с молчаливой потерей текста. В подтверждённо well-formed XML литеральный<!--в значении обязан быть entity-экранирован, а CDATA drawio не использует → под гейтом регулярка попадает только в настоящие узлы-комментарии.How verified
<!--в значении переживает round-trip; многострочные/множественные комментарии; backstopno-commentsпри прямом заходе вlintModel.true→ malformed-тест краснеет (исходный data-loss возвращается) — оба восстановлены.drawio-xml.ts+ тест.Осознанные SUGGESTION (не блокеры)
lintModelкидает пачку, где рядом сwell-formed-xmlесть и[no-comments]— формально шумно, но вход корректно отклонён, усечения нет.Checklist
Ревью — drawio авто-стрип XML-комментариев (#505). Вердикт: changes-requested
Подход правильный и хорошо защищённый: стрип гейтится well-formedness — атрибут-вектор (сырой
<в value) закрыт (эмпирически:parseXmlреджектит → НЕ стрипается → value цел, #521-класс закрыт), regex безопасен (non-greedy, ReDoS-linear,---тело гейтится), happy-path байт-в-байт,no-commentsbackstop жив, тесты не вакуумны (мутация gate → тест #24 краснеет). Security/regressions/conventions/doc/simplification/architecture — LGTM. Эскалаций нет.Открыто:
<!-- -->внутри CDATA (вектор, который «no CDATA»-премиса дока не покрывает).📋 F1 — детали + эмпирика
F1 [warning · stability] тихая порча внутри CDATA —
packages/mcp/src/lib/drawio-xml.ts:1066(премиса) +:1078-1085(гейт)Safety-аргумент опирается на два тезиса: (1) well-formed XML запрещает голый
<в value — верно, подтверждено; (2) коммент:1066«drawio XML uses no CDATA sections, so nothing but a comment node can hold the<!-- ... -->sequence» — НЕ enforced и эмпирически ложен. CDATA-секция well-formed →parseXmlотдаётerror===null→ гейт пропускает →/<!--[\s\S]*?-->/gматчит литеральный<!--...-->ВНУТРИ CDATA-текста и удаляет его.Воспроизвёл сам через собранный модуль: вход
<foo><![CDATA[keep<!--x-->keep]]></foo>→modelXml=<![CDATA[keepkeep]]>+ вводящий в заблуждениеwarnings:["stripped 1 XML comment(s)"]. Тихая потеря контента, без ошибки, с ложным «успехом». Это #521-класс через вектор, который гейт не покрывает — и который собственная джустификация изменения объявляет невозможным.Reachability низкая (drawio-лейблы живут в
value=-атрибутах, где CDATA невозможна, а<!--обязан быть экранирован; модели надо спонтанно выдать CDATA с comment-синтаксисом) — потому warning, не critical. Но это реальный silent-corruption путь с бесплатным однострочным фиксом, и backstop тут НЕ спасает (кейс проходит как «успех», not an error).Fix: расширить гейт — пропускать стрип при наличии CDATA:
const hasCdata = rawModel.includes("<![CDATA[");commentMatches = (parseError === null && !hasCdata) ? (rawModel.match(/<!--[\s\S]*?-->/g) ?? []) : []. Реальный drawio CDATA не использует → на легитимных моделях бесплатно; CDATA-модель с настоящим комментом падает на удержанныйno-commentsbackstop вместо тихой порчи. Обнови коммент:1066: CDATA теперь ЯВНО исключён, а не «предполагается отсутствующим». Добавь тест: CDATA с<!--внутри → НЕ стрипается (значение цело).9 аспектов (2 агента + мой разбор крюкса + эмпирический CDATA-репро). Гейт: mcp node:test 865/865 (+79 тестов зелёные). Атрибут-вектор gated+tested (мутация gate → тест #24 краснеет). Дерево чисто. База develop
03eafa6c.Модель органически вставляет `<!-- ... -->` в mxGraph-XML вопреки запрету в описании тула (природа LLM, промптингом не лечится). Жёсткая ошибка линтера [no-comments] заставляла её ПОЛНОСТЬЮ перегенерировать диаграмму — впустую потраченный tool-call на почти каждой первой генерации. Теперь в общем prepare-пути (`prepareModel`, через который идут drawioCreate/drawioUpdate/drawioEditCells) комментарии срезаются ДО линта: `xml.replace(/<!--[\s\S]*?-->/g, "")`. Комментарии не несут семантики, так что стрип безопасен. Число срезанных уходит в `warnings[]` как `stripped N XML comment(s)` (только при N > 0) — модель это видит, но НЕ ретраит. Стрип ГЕЙТИТСЯ двумя условиями, чтобы регэксп бил только по настоящим comment-нодам: 1. well-formedness — сначала парсим модель; на malformed-входе (сырой неэкранированный `<!--` внутри значения атрибута на пути create/update, где normalizeInput отдаёт строку без парсинга) стрип НЕ выполняется, иначе он молча вырезал бы текст автора. Вход отклоняют правила value-escaping / well-formed-xml, автор видит реальную ошибку. 2. отсутствие CDATA — CDATA-секция well-formed, но её текст может содержать литеральный `<!-- ... -->`, который НЕ является comment-нодой; стрип молча удалил бы контент автора. Настоящий drawio держит подписи в атрибутах `value=` (CDATA невозможен), так что исключение CDATA бесплатно на легитимных моделях; CDATA-модель с реальным комментарием падает на сохранённый backstop no-comments (явная ошибка), а не тихо портится. Правило `no-comments` в линтере оставлено как defense-in-depth backstop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>fe30936e2fto1f2999e5adОтвет на ревью (F1)
F1 [warning · stability] — тихая порча
<!-- -->внутри CDATA — закрыто.Причина дыры: strip гейтился только на well-formedness. CDATA-секция well-formed →
parseXmlотдаётerror===null→ гейт пропускал →/<!--[\s\S]*?-->/gматчил литеральный<!--...-->внутри CDATA-текста и удалял его, рапортуя ложныйstripped 1 XML comment(s). Твой репро<foo><![CDATA[keep<!--x-->keep]]></foo>→<![CDATA[keepkeep]]>воспроизведён.Фикс (
drawio-xml.ts,prepareModel~:1081–1085): добавлен CDATA-суб-гейт.hasCdata = rawModel.includes("<![CDATA[")считается на сырой модели; strip выполняется только приparseError === null && !hasCdata. Модель с CDATA теперь не стрипается вовсе — если в ней есть настоящий comment-node, срабатывает сохранённый бэкстопno-comments(явнаяDrawioLintError) вместо тихой потери. Реальные drawio-лейблы живут вvalue=-атрибутах (там HTML-escaped, CDATA невозможна), так что исключение CDATA бесплатно на легитимных моделях.Комментарий-премиса (~:1073–1079) переписан: CDATA теперь документируется как явное второе условие гейта (не «предполагается отсутствующей»), с честным описанием трейд-оффа.
Тест (
drawio-xml.test.mjs): well-formed модель с<![CDATA[keep<!-- x -->keep]]>→prepareModelбросает (ассертит срабатываниеno-comments-бэкстопа, а не просто отсутствие strip-варнинга) — сильный тест на факт-порчи, а не на её симптом.Верификация:
@docmost/mcpпересобран (tsc чисто); drawio-сьют 41/41. Мутация суб-гейта (снять!hasCdata) → краснеет ровно CDATA-тест (34/1), тихая порча возвращается; восстановлено → 41/0. Скоуп коммита:drawio-xml.ts+ тест, 0 node_modules. Ветка отребейзена на develop (17 коммитов), конфликтов нет.внутренний цикл: 1 проход внутреннего ревью на этот фикс — APPROVE, изоляция подтверждена мутацией (краснеет только целевой тест), общий strip-путь не сломан.
Новый head:
1f2999e5.Ре-ревью — drawio comment-strip (#505), раунд 2. Вердикт: approved ✅
F1 (тихая порча внутри CDATA) закрыт, проверил адверсариально:
!hasCdata(drawio-xml.ts:1108-1110:parseError === null && !hasCdata,hasCdata = rawModel.includes("<![CDATA[")), коммент:1100-1104исправлен — CDATA теперь ЯВНО исключён, не «предполагается отсутствующим».<foo><![CDATA[keep<!--x-->keep]]></foo>→ теперь БРОСАЕТ[no-comments]lint-ошибку (падает на удержанный backstop), а НЕ молча стрипает вkeepkeep. Контент цел, ложного «успеха» нет. Контроль (настоящий comment-узел без CDATA) по-прежнему стрипается корректно.<!-- -->inside a CDATA section is NOT stripped» (drawio-xml.test.mjs:311); мутация (снял&& !hasCdata) → тест краснеет (not ok 25). Не вакуумен.Гейт: mcp node:test 872/872. Дерево после мутаций чисто. Готово к мержу.