fix(notifications): HTML-safe card data render — fix <1м injection freezing the tracker (ORCH-095) #107
@@ -3,6 +3,13 @@
|
||||
Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу.
|
||||
|
||||
## [Unreleased]
|
||||
- **Live-карточка трекера: HTML-инъекция «<1м» больше не застывает карточку — экранирование всех данных-полей на границе рендера** (ORCH-095, `fix`): карточка задачи (`src/notifications.py::render_task_tracker`) шлётся/редактируется с `parse_mode=HTML`. `_fmt_minutes` для стадии < 60 с возвращает литерал `"<1м"`, который интерполировался в HTML-текст **сырым** → Telegram парсит `<1м` как открывающий тег → `editMessageText` отвечает `400 can't parse entities: Unsupported start tag "1м"` → `edit_telegram` классифицирует как `EDIT_FAILED` → `update_task_tracker` делает ранний `return` (анти-дубль ORCH-087) → **карточка застывает** (детерминированно воспроизведено 09.06 на ORCH-093, `message_id 18854`). Корневой класс шире одного `<1м`: все подставляемые **данные** (длительности, статус-лейбл, модель, эффорт, токены/стоимость) вставлялись сырыми; экранирован был только заголовок (`esc_title`) и href/label внутри `plane_issue_link`. **Аддитивно, never-raise, без нового поведения конвейера:** `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / транспорт нотификаций / схема БД — **не тронуты** (затронут ровно один модуль индикативного слоя); kill-switch не требуется (исправление дефекта корректности, откат = `git revert`).
|
||||
- **Экранирование на границе рендера, не в источнике (ADR-001 D1/D2, AC-1/AC-2):** новый модуль-локальный хелпер `_esc(x) = html.escape(str(x))` (never-raise → `""` на исключении) оборачивает каждое подставляемое **данные-значение** (категория D) ровно один раз в точке интерполяции в `render_task_tracker`/`_stage_line`: длительности (`_fmt_minutes`/`_capped_review_str`), статус-лейбл (`_card_status_label`), модель (`short_model_name`), эффорт (`_run_effort`), токены/стоимость (`fmt_tokens`/`fmt_cost`). Функции-источники остаются **HTML-агностичными** (данные, не разметка): `src/usage.py` и `_fmt_minutes` не тронуты — `_fmt_minutes` продолжает возвращать `"<1м"`, безопасность даёт escape на границе (`<1м` рендерится оператору визуально идентично `<1м` → видимый формат не меняется).
|
||||
- **Категория M (намеренная разметка) неприкосновенна (D5, AC-3):** кликабельный номер задачи `num_html` (`plane_issue_link`, внутри уже экранированы href+label), `link_for(...)` в строке «⏳ ждёт …», `_done_link(...)` («🔗 PR #n · 📦 Внедрено») и уже-экранированный `esc_title` через `_esc` **не** проходят → остаются валидным HTML, номер остаётся кликабельным. Двойное экранирование (`&lt;`) структурно исключено: D-слот → `_esc` ровно один раз, M-слот → as-is.
|
||||
- **Defence-in-depth (D3):** экранируются и сейчас-безопасные D-поля (токены/стоимость/модель дают только цифры/`.`/`k`/`M`/`$`/`^claude-…$`) — escape для них no-op, выгода — структурный инвариант «каждый D-слот экранирован», устойчивый к будущей смене формата источника.
|
||||
- **Восстановление застрявших карточек (D4, AC-4):** механизм — достаточное условие FR-4 без нового кода: на ближайшем переходе стадии `update_task_tracker` рендерит новый безопасный текст → `edit_telegram` отвечает `200` → застрявшая карточка обновляется на месте. Переклассификация `can't parse entities` → переотправка **отвергнута** (после фикса источник из наших данных устранён структурно; касание ветки `EDIT_FAILED`/леджера рискует анти-дублем ORCH-087). Known-limitation (унаследовано ORCH-087/Telegram-48ч): карточка задачи, завершившейся до деплоя фикса, не восстанавливается (нет будущего рендера).
|
||||
- **Трассировка:** перед правкой блоков, помеченных ORCH-042/067/087/091, прочитаны их ADR — инварианты (одна карточка на задачу, леджер сирот + анти-дубль, отражение откатов + суммирование `_stage_line`, строка Plane-статуса/кликабельный номер) сохранены по построению (ORCH-095 лишь оборачивает уже вычисленные D-значения в `_esc`, не меняя состав строк/порядок/логику подавления).
|
||||
- Тесты: новый `tests/test_tracker_html_escape.py` (TC-01..TC-11: sub-minute escape на границе, never-raise `_fmt_minutes`/`_esc` на граничных входах, рендер sub-minute без сырого `<1м`, заголовок со спецсимволами без двойного экранирования, escape статус-лейбла/модели/эффорта, HTML-безопасность токенов/стоимости, регресс кликабельного `<a href>` номера и `_done_link`, parse-safe edit-payload, edit-in-place без новой карточки + анти-дубль на транзиентном фейле, never-raise на битых входах). Полный регресс `tests/ -q` зелёный (1437). ADR: `docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md`. Откат: `git revert` (один модуль + тесты + CHANGELOG, без миграций/kill-switch).
|
||||
- **Терминальная (done) задача держит `Done` в Plane: terminal-window-aware гард deploy-статусов** (ORCH-094, `fix`): задача с БД `stage=done` и 0 активных job'ов (верифицировано на ORCH-061, task 47) стабильно флаппила в Plane `Awaiting Deploy ⟷ Monitoring after Deploy` (273 активности парами, само не затихает) вместо `Done`. Корень: три deploy-фазовых сеттера (`set_issue_awaiting_deploy`/`set_issue_deploying`/`set_issue_monitoring`) **терминал-слепы** — любой стейл/двойной/неизвестный вызов под бот-токеном перезаписывает `Done` промежуточным deploy-статусом, и обратно, бесконечно. **Аддитивно, never-raise, под kill-switch, в зоне self-hosting:** `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / machine-verdict ключи (`deploy_status:`/`staging_status:`/…) / схема БД — **не тронуты** (читается существующая `tasks.stage`, без миграции).
|
||||
- **Единый гард на низком чокпоинте (FR-2, D1/D2):** новый leaf `src/deploy_status_guard.py` (чистая, never-raise, config-gated логика; по образцу `serial_gate.py`/`labels.py`/`cancel.py`) — `decide(work_item_id, target, reason) -> ALLOW | CONVERGE_DONE | SUPPRESS`. Гард ставится на **входе** трёх сеттеров `plane_sync` (а не в caller'ах `stage_engine`) → перехватывает **любой** путь, включая неизвестный актор под бот-токеном. Предикат легитимности: deploy-статус легитимен ⇔ задача **нетерминальна** ИЛИ (`done` **И** активно пост-деплой-окно `post_deploy.window_active` = ARMED & не DONE). Для `done`: `monitoring`+окно-активно → `ALLOW`; иначе → `CONVERGE_DONE` (сеттер вместо PATCH'а зовёт `set_issue_done`, идемпотентно). `cancelled` → `SUPPRESS` (не штампуем поверх терминала ORCH-090). Нетерминальная задача → `ALLOW` (рабочий deploy-цикл 1:1, AC-4). Task не найден / не-self репо / kill-switch off / любое исключение → `ALLOW` (fail-safe к прежнему поведению 1:1, NFR-1).
|
||||
- **Перенос арм-блока перед terminal-sync (D3, AC-4):** в `advance_stage` (ветка `next_stage=="done"`) блок `post_deploy.arm_monitor` перемещён **выше** блока `set_issue_monitoring` (стр. 404). Критично: `update_task_stage(task_id,"done")` пишет `stage='done'` **раньше** легитимного первого `Monitoring` — без переноса гард ошибочно свёл бы его к Done. Арм-первым пишет `ARMED` → `window_active==True` → `ALLOW` пропускает легитимный `Monitoring`; re-drive `deploy→done` **после** закрытия окна (`DONE` present) → `window_active==False` → `CONVERGE_DONE` (не воскрешает `Monitoring`). Перенос безопасен: `arm_monitor` лишь пишет sentinel + ставит отложенный job, не зависит от Plane-статуса/merge-lease (release остаётся после terminal-sync). Инварианты ORCH-021 (идемпотентный арм по `ARMED`) и ORCH-066 (`deploy→done` self ⇒ `Monitoring`) сохранены.
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -142,6 +142,8 @@ claude.exe --print --system-prompt --allowedTools Read,Write,Edit,Bash
|
||||
|
||||
**Кликабельный номер задачи (ORCH-067).** Номер в заголовке карточки И во всех уведомлениях орка, где упоминается `work_item_id`, — HTML-ссылка на issue в Plane через общий `plane_issue_link` / `link_for` (URL строит `_plane_issue_url` с loopback/workspace/project-гардами, переиспользуя резолв ORCH-017). Fail-safe: при нехватке любого из (web-base/не-loopback, workspace, project_id, plane_issue_id) → `html.escape(work_item_id)` без `<a>`; динамические части экранируются, `<a>`-разметка валидна под `parse_mode=HTML`. Алерты `stage_engine`/`launcher`/`security_gate`/`reconciler` переведены на `link_for` (резолвит `repo`+`plane_issue_id` из БД по `task_id` или `work_item_id`).
|
||||
|
||||
**HTML-безопасность данных карточки (ORCH-095).** Текст карточки шлётся с `parse_mode=HTML` и собирается из слотов двух категорий: **markup** (намеренная разметка — `num_html`/`plane_issue_link`, `link_for(...)`, `_done_link(...)`, уже-экранированный `esc_title`) и **data** (подставляемые значения — длительности `_fmt_minutes`/`_capped_review_str`, статус-лейбл `_card_status_label`, имя модели `short_model_name`, эффорт `_run_effort`, токены/стоимость `fmt_tokens`/`fmt_cost`). Инвариант: **каждый data-слот экранируется `html.escape` ровно один раз на границе рендера** (`render_task_tracker`/`_stage_line`); функции-источники остаются HTML-агностичными, markup-слоты не экранируются (двойное экранирование запрещено). Это устранило класс «неэкранированные данные в HTML-тексте»: до фикса `_fmt_minutes(<60s)` возвращал литерал `<1м`, который Telegram парсил как открывающий тег → `editMessageText` `400 can't parse entities` → `EDIT_FAILED` → ранний `return` (анти-дубль ORCH-087) → карточка застывала (инцидент ORCH-093). `_fmt_minutes` по-прежнему возвращает `<1м` — escape на границе (`<1м`) рендерит его визуально идентично; формат не меняется. Застрявшая (в окне) карточка авто-восстанавливается следующим безопасным рендером; `edit_telegram`/`update_task_tracker`/леджер сирот/режимы `bump`/`edit` не тронуты. Детали — [ORCH-095 ADR-001](../work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md).
|
||||
|
||||
## Database Schema
|
||||
|
||||
```sql
|
||||
|
||||
7
docs/work-items/ORCH-095/00-business-request.md
Normal file
7
docs/work-items/ORCH-095/00-business-request.md
Normal file
@@ -0,0 +1,7 @@
|
||||
# Business Request: BUG: карточка трекера застывает — HTML-инъекция «<1м» в render_task_tracker (parse_mode=HTML)
|
||||
|
||||
Work Item ID: ORCH-095
|
||||
|
||||
## Description
|
||||
|
||||
TBD
|
||||
154
docs/work-items/ORCH-095/01-brd.md
Normal file
154
docs/work-items/ORCH-095/01-brd.md
Normal file
@@ -0,0 +1,154 @@
|
||||
---
|
||||
work_item: ORCH-095
|
||||
stage: analysis
|
||||
author_agent: analyst
|
||||
status: ready-for-review
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# 01 — BRD (бизнес-требования): ORCH-095 — HTML-инъекция «<1м» в render_task_tracker застывает live-карточку
|
||||
|
||||
Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis
|
||||
|
||||
## 1. Бизнес-контекст и проблема
|
||||
|
||||
Live-трекер задачи (`src/notifications.py::render_task_tracker`) — **основной канал
|
||||
видимости конвейера для оператора**. Слава узнаёт состояние каждой задачи по её единственной
|
||||
карточке в Telegram (инвариант «одна карточка на задачу», ORCH-042/067/087). Если карточка
|
||||
перестаёт обновляться — оператор слепнет: задача реально идёт/завершилась, а карточка врёт.
|
||||
|
||||
**Установленный факт (воспроизведён детерминированно 09.06, сырой ответ Telegram).**
|
||||
Прямой вызов `editMessageText` для застрявшей карточки ORCH-093 (`message_id 18854`) вернул:
|
||||
|
||||
```
|
||||
400 Bad Request: can't parse entities: Unsupported start tag "1м" at byte offset 500
|
||||
```
|
||||
|
||||
В тексте карточки на позиции ~379 присутствует подстрока `<1м · …` — длительность стадии
|
||||
«меньше одной минуты», которую `_fmt_minutes` (`src/notifications.py:288-289`) рендерит как
|
||||
литерал **`<1м`**. Карточка отправляется с `parse_mode=HTML` (`editMessageText`,
|
||||
`notifications.py:175`). Telegram трактует `<1м` как **открывающий HTML-тег** → парсинг падает
|
||||
с `400` → `edit_telegram` возвращает `EDIT_FAILED` → `update_task_tracker` по ветке
|
||||
`EDIT_FAILED` (`notifications.py:733-739`) делает `return`, **не** отправляя новую карточку
|
||||
(защита от дублей, ORCH-087) → карточка **застывает** на стейте, где `<1м` впервые попал в текст.
|
||||
|
||||
**Цепочка отказа** (по коду):
|
||||
`_fmt_minutes(<60s) → "<1м"` → интерполируется в HTML без экранирования → `editMessageText`
|
||||
`400 can't parse entities` → `edit_telegram → EDIT_FAILED` → `update_task_tracker` ранний
|
||||
`return` → карточка не обновляется до конца жизни задачи.
|
||||
|
||||
**Почему проявляется не на каждой задаче.** Баг ловится **только** когда хотя бы одна
|
||||
длительность стадии < 1 мин (`seconds < 60`) и эта строка попадает в текст, который затем
|
||||
редактируется. Карточки ORCH-090/091 редактировались успешно (на момент `edit` в их тексте
|
||||
`<1м` не было); ORCH-093 — упала. Это объясняет «плавающую» природу симптома.
|
||||
|
||||
**Корневой класс дефекта — шире одного `<1м`.** Текст карточки собирается с `parse_mode=HTML`
|
||||
из смеси (а) намеренной разметки-обёртки (`<a href>` номер задачи, `<b>`) и (б) подставляемых
|
||||
**данных**. Намеренная разметка экранироваться **не должна**; данные — должны. Сейчас
|
||||
экранирован только заголовок (`esc_title`, `notifications.py:428`) и href/label внутри
|
||||
`plane_issue_link`. Прочие данные — длительности (`_fmt_minutes`), метрики токенов/стоимости
|
||||
(`fmt_tokens`/`fmt_cost`), имя модели (`short_model_name`), статус-лейбл
|
||||
(`_card_status_label`) — вставляются **без** `html.escape`. `<1м` — первый сработавший
|
||||
экземпляр этого класса; задача закрывает класс, а не единичный символ.
|
||||
|
||||
## 2. Объём (scope)
|
||||
|
||||
### В объёме
|
||||
- Устранить HTML-инъекцию в `render_task_tracker`: любые **данные**, попадающие в текст
|
||||
карточки с `parse_mode=HTML`, не должны ломать парсер Telegram (`< > &` в данных
|
||||
безопасны).
|
||||
- Привести формат «длительность < 1 мин» к HTML-безопасному виду (экранированный `<1м`
|
||||
ИЛИ переформулировка `<1м` → `~0м` / `< 1 мин` с экранированием).
|
||||
- Сохранить работоспособность **намеренной** разметки карточки (`<a href>` номер задачи,
|
||||
жирный/прочее форматирование) — экранируются только данные, не обёртка.
|
||||
- Восстановить обновления уже застрявших карточек (после фикса карточка возобновляет
|
||||
обновления или переотправляется свежей).
|
||||
- Юнит-покрытие HTML-безопасности всех динамических полей; зелёный регресс `pytest tests/ -q`;
|
||||
запись в `CHANGELOG.md`.
|
||||
|
||||
### Вне объёма
|
||||
- Изменение `STAGE_TRANSITIONS`, `QG_CHECKS`, `check_*`, схемы БД — **не трогаются** (баг
|
||||
чисто в слое рендера уведомлений).
|
||||
- Изменение режима трекера (`bump`/`edit`), логики леджера сирот (ORCH-087), статусной модели
|
||||
ORCH-066, транспортных примитивов (`send_telegram`/`edit_telegram`/`delete_telegram`) —
|
||||
кроме точечной HTML-безопасности самого текста.
|
||||
- Редизайн раскладки/состава карточки, новые метрики, перевод строк.
|
||||
- Изменение машинных вердиктов / frontmatter-контракта.
|
||||
|
||||
## 3. Заинтересованные стороны
|
||||
|
||||
- **Заказчик / репортёр:** Слава (оператор) — обнаружил баг 09.06 (карточка ORCH-093 застряла,
|
||||
«по 91 уже нету»).
|
||||
- **Затронуты:** все наблюдатели Telegram-трекера по **всем** проектам (self-hosting: общий
|
||||
прод-инстанс обслуживает и enduro-trails — карточки их задач так же уязвимы при стадии < 1 мин).
|
||||
- **Принимает результат:** reviewer/tester конвейера ORCH; финальная приёмка — оператор
|
||||
(карточки снова обновляются в реальном времени).
|
||||
|
||||
## 4. Бизнес-требования (BR)
|
||||
|
||||
- **BR-1** — Карточка трекера, в тексте которой есть стадия длительностью < 1 мин, должна
|
||||
успешно редактироваться (`editMessageText` → `200`, не `400 can't parse entities`). Источник
|
||||
отказа — литерал `<1м` от `_fmt_minutes` — устранён. (⇒ G1, G2)
|
||||
- **BR-2** — **Все** динамические значения, вставляемые в текст карточки с `parse_mode=HTML`
|
||||
(длительности, метрики токенов/стоимости, имя модели/эффорта, имена/лейблы стадий,
|
||||
статус-лейбл, заголовок задачи), HTML-безопасны: символы `< > &` в **данных** не
|
||||
интерпретируются Telegram как разметка. (⇒ G1)
|
||||
- **BR-3** — Длительность «меньше минуты» рендерится так, чтобы не выглядеть открывающим
|
||||
HTML-тегом: экранированный `<1м` **ИЛИ** переформулировка (`~0м` / `< 1 мин`) с
|
||||
экранированием. Видимое оператору значение остаётся осмысленным («меньше минуты»). (⇒ G2)
|
||||
- **BR-4** — **Регресс намеренной разметки:** кликабельный номер задачи (`<a href>`,
|
||||
`plane_issue_link`) и любое форматирование-обёртка (`<b>` и т.п.) продолжают рендериться и
|
||||
оставаться кликабельными/валидными — экранируются только подставляемые данные, не разметка. (⇒ G3)
|
||||
- **BR-5** — Уже застрявшая карточка (класс ORCH-093) после деплоя фикса **возобновляет
|
||||
обновления**: либо успешный `editMessageText` на следующем переходе стадии, либо
|
||||
переотправка свежей карточки. Конкретный механизм восстановления (текст снова валиден →
|
||||
edit проходит, ИЛИ классификация `can't parse entities` как пересоздаваемой) — решение
|
||||
архитектора; бизнес-требование — карточка перестаёт быть «замёрзшей сиротой». (⇒ G... / AC-4)
|
||||
|
||||
## 5. Нефункциональные требования (NFR)
|
||||
|
||||
- **NFR-1 (never-raise):** `render_task_tracker` и весь путь уведомлений сохраняют контракт
|
||||
«никогда не роняют конвейер» — любая ошибка рендера/экранирования деградирует к
|
||||
fallback-строке, не исключение.
|
||||
- **NFR-2 (нулевая регрессия разметки):** существующие зелёные тесты трекера
|
||||
(`test_telegram_tracker.py`, `test_tracker_*`, `test_notifications_orphans.py`,
|
||||
`test_notify_issue_links.py`) остаются зелёными; кликабельность номера и формат строк не
|
||||
деградируют визуально (кроме намеренной смены вида «<1м»).
|
||||
- **NFR-3 (self-hosting):** фикс — изменение **только** слоя рендера уведомлений; прод-контейнер
|
||||
`orchestrator` не перезапускается в рамках стадий разработки; обязательна страховка
|
||||
`deploy-staging` перед прод-деплоем. Машина стадий/гейты/схема БД не затрагиваются.
|
||||
- **NFR-4 (совместимость):** изменение обратносовместимо по данным/схеме; не требует миграций;
|
||||
применяется к новым рендерам сразу после деплоя.
|
||||
|
||||
## 6. Допущения и ограничения
|
||||
|
||||
- Карточка всегда отправляется с `parse_mode=HTML` (`send_telegram:58`, `edit_telegram:175`) —
|
||||
это инвариант (ссылки/жирный требуют HTML); переход на `parse_mode=None`/MarkdownV2 **не**
|
||||
рассматривается (сломает намеренную разметку, шире объёма).
|
||||
- `fmt_tokens`/`fmt_cost` сейчас выдают только цифры/`.`/`k`/`M`/`$` (HTML-безопасно), но
|
||||
требование BR-2 покрывает их **defence-in-depth** на случай будущих изменений формата.
|
||||
- Telegram-лимит 48ч: карточки старше 48ч физически неудаляемы/неперезаписываемы — для них
|
||||
восстановление недостижимо (known-limitation, унаследовано от ORCH-087); BR-5 относится к
|
||||
карточкам в пределах окна.
|
||||
- Источник `<1м` — `_fmt_minutes` (единственная функция, эмитящая литерал `<`); прочие данные
|
||||
лишь потенциально опасны. Точка(и) внесения экранирования — решение архитектора (централизовать
|
||||
в `_fmt_minutes`/на точке рендера/обёрткой-хелпером).
|
||||
|
||||
## 7. Критерии успеха
|
||||
|
||||
Карточка задачи со стадией < 1 мин успешно редактируется (нет `400 can't parse entities`);
|
||||
все динамические поля HTML-безопасны; намеренная разметка (ссылка-номер, форматирование)
|
||||
рендерится и кликабельна; застрявшие карточки возобновляют обновления; `never-raise` сохранён;
|
||||
`pytest tests/ -q` зелёный; `CHANGELOG.md` обновлён. Детальные PASS/FAIL — `03-acceptance-criteria.md`.
|
||||
|
||||
## 8. Риски
|
||||
|
||||
- **Двойное экранирование** уже экранированных полей (`esc_title`, href/label в
|
||||
`plane_issue_link`) → `&lt;` в выводе. Митигировать на стадии архитектуры (экранировать
|
||||
ровно один раз на источник данных).
|
||||
- **Случайное экранирование разметки-обёртки** (`<a>`, `<b>`) → ссылки/жирный перестают
|
||||
работать (регресс BR-4). Чёткая граница «данные vs обёртка».
|
||||
- Изменение вида «<1м» меняет визуал карточки — согласовать формулировку с оператором (BR-3
|
||||
допускает оба варианта).
|
||||
- Детали/перечень — `10-tech-risks.md` (заполняет архитектор).
|
||||
132
docs/work-items/ORCH-095/02-trz.md
Normal file
132
docs/work-items/ORCH-095/02-trz.md
Normal file
@@ -0,0 +1,132 @@
|
||||
---
|
||||
work_item: ORCH-095
|
||||
stage: analysis
|
||||
author_agent: analyst
|
||||
status: ready-for-review
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# 02 — ТЗ (TRZ): ORCH-095 — HTML-безопасность динамических полей render_task_tracker
|
||||
|
||||
Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis
|
||||
|
||||
> ТЗ описывает **конкретные изменения к реализации**, выведенные из BRD и фактического кода.
|
||||
> Архитектурное обоснование/выбор точки внесения экранирования — задача архитектора (06-adr).
|
||||
|
||||
## 1. Сводка изменения
|
||||
|
||||
Текст live-карточки (`render_task_tracker`) собирается с `parse_mode=HTML` из намеренной
|
||||
разметки-обёртки (`<a href>` номер задачи, форматирование) и подставляемых **данных**. Сейчас
|
||||
экранирован только заголовок (`esc_title`) и href/label внутри `plane_issue_link`; остальные
|
||||
данные вставляются сырыми. Литерал `<1м` (длительность < 1 мин), возвращаемый `_fmt_minutes`,
|
||||
Telegram парсит как открывающий тег → `editMessageText` падает `400 can't parse entities` →
|
||||
`edit_telegram → EDIT_FAILED` → `update_task_tracker` делает ранний `return` → карточка
|
||||
застывает.
|
||||
|
||||
Требуется: (а) сделать формат «< 1 мин» HTML-безопасным; (б) гарантировать HTML-безопасность
|
||||
**всех** данных, попадающих в текст карточки, **не** экранируя намеренную разметку-обёртку;
|
||||
(в) обеспечить возобновление обновлений ранее застрявших карточек. Изменение локализовано в
|
||||
слое уведомлений; машина стадий/гейты/схема БД не затрагиваются.
|
||||
|
||||
## 2. Задействованные модули / пути
|
||||
|
||||
| Путь | Действие |
|
||||
|------|----------|
|
||||
| `src/notifications.py` | **изменить** — `_fmt_minutes` (~280) и/или точки рендера в `render_task_tracker` (~355): HTML-безопасность данных |
|
||||
| `src/notifications.py::render_task_tracker` | **изменить** — экранировать данные: длительности (`dur`), `status_label`, `model`/`effort`, метрики (defence-in-depth); НЕ трогать `num_html`, `_done_link`-разметку |
|
||||
| `src/notifications.py::_card_status_label` (~1173) | **проверить/экранировать на потребителе** — статус-лейбл вставляется в `status_line` сырым |
|
||||
| `src/notifications.py::edit_telegram` (~157) | **возможно изменить** (на усмотрение архитектора) — классификация `can't parse entities` для восстановления застрявших карточек (BR-5/AC-4) |
|
||||
| `src/notifications.py::update_task_tracker` (~650) | **возможно затронуть** — ветка `EDIT_FAILED` vs пересоздание при перманентном parse-фейле (BR-5/AC-4) |
|
||||
| `tests/test_telegram_tracker.py` (или новый `tests/test_tracker_html_escape.py`) | **создать/дополнить** — юнит HTML-безопасности всех динамических полей |
|
||||
| `CHANGELOG.md` | **изменить** — запись о фиксе |
|
||||
|
||||
> Примечание: `fmt_tokens`/`fmt_cost`/`short_model_name` живут в `src/usage.py`; их выход
|
||||
> сейчас HTML-безопасен (цифры/`.`/`k`/`M`/`$`/имя модели). Менять `src/usage.py` **не
|
||||
> требуется** — defence-in-depth экранирование делается на потребителе в `notifications.py`.
|
||||
|
||||
## 3. Функциональные требования
|
||||
|
||||
### FR-1 — HTML-безопасный формат «меньше минуты» (⇒ BR-1, BR-3)
|
||||
Длительность стадии < 60 с не должна порождать подстроку, которую Telegram трактует как
|
||||
открывающий тег. Текущий `_fmt_minutes(seconds)` при `0 < seconds < 60` возвращает литерал
|
||||
`"<1м"` (`notifications.py:288-289`). Поведение должно стать одним из (выбор — архитектор):
|
||||
- экранированный вывод `<1м` (видится оператору как `<1м`), **либо**
|
||||
- переформулировка `~0м` / `< 1 мин` с последующим экранированием.
|
||||
Инвариант: для **любого** входа `_fmt_minutes` (включая `0м`, `Nм`, `~Nм` от
|
||||
`_capped_review_str`) результат, попав в `parse_mode=HTML`, не ломает парсер. `_fmt_minutes`
|
||||
сохраняет never-raise (нечисловой/None вход → `0м`).
|
||||
|
||||
### FR-2 — HTML-безопасность всех данных карточки (⇒ BR-2)
|
||||
Каждое **подставляемое значение-данные**, попадающее в текст `render_task_tracker`,
|
||||
экранируется `html.escape(...)` ровно один раз перед вставкой в HTML-текст. Перечень полей-данных:
|
||||
|
||||
| Поле | Источник | Текущий статус |
|
||||
|------|----------|----------------|
|
||||
| Заголовок задачи | `title` → `esc_title` | уже экранирован ✓ (не дублировать) |
|
||||
| Длительности стадий / BRD / done | `_fmt_minutes`, `_capped_review_str` | **дыра** (FR-1) |
|
||||
| Статус-лейбл карточки | `_card_status_label` → `status_label` | **дыра** — экранировать |
|
||||
| Имя модели | `short_model_name(last["model"])` | экранировать (defence-in-depth) |
|
||||
| Эффорт | `_run_effort(last)` | экранировать (defence-in-depth) |
|
||||
| Токены / стоимость | `fmt_tokens`/`fmt_cost` | HTML-безопасны; экранировать defence-in-depth |
|
||||
| Метка «попытка N» / лейблы стадий | статические константы `_TRACKER_STAGES`/`_BRD_LABEL` | статичны; не требуют, но безопасно |
|
||||
|
||||
Инвариант FR-2: после рендера **ни один** символ `< > &`, пришедший из данных, не остаётся
|
||||
неэкранированным в выходном тексте.
|
||||
|
||||
### FR-3 — Сохранность намеренной разметки-обёртки (⇒ BR-4)
|
||||
Намеренные HTML-фрагменты **не** экранируются:
|
||||
- `num_html` = `plane_issue_link(...)` — кликабельный `<a href>` номер задачи (внутри уже
|
||||
экранированы href через `html.escape(url, quote=True)` и label);
|
||||
- `link_for(...)` в строке «⏳ ждёт …» — намеренные ссылки;
|
||||
- `_done_link(...)` — строка `🔗 PR #n · 📦 Внедрено`.
|
||||
После фикса эти фрагменты рендерятся как валидный HTML и остаются кликабельными. Запрещено
|
||||
двойное экранирование уже экранированных полей (`esc_title`, внутренности `plane_issue_link`).
|
||||
|
||||
### FR-4 — Возобновление обновлений застрявших карточек (⇒ BR-5)
|
||||
После деплоя фикса карточка, ранее застрявшая на `400 can't parse entities`, должна
|
||||
возобновить обновления. Достаточное условие по умолчанию: текст следующего рендера больше не
|
||||
содержит небезопасной подстроки → `editMessageText` проходит (`200`) на ближайшем переходе
|
||||
стадии. Опционально (решение архитектора): классифицировать перманентный parse-фейл в
|
||||
`edit_telegram`/`update_task_tracker` как повод **переотправить** свежую карточку вместо
|
||||
тихого `return` по `EDIT_FAILED` — но **без** регресса защиты от дублей (ORCH-087: транзиентные
|
||||
фейлы по-прежнему НЕ плодят карточки). Если выбирается переклассификация — она должна отличать
|
||||
перманентный `can't parse entities` от транзиентного (network/timeout/5xx).
|
||||
|
||||
### FR-5 — never-raise (⇒ NFR-1)
|
||||
Все изменённые функции сохраняют контракт «никогда не роняют конвейер»: ошибка
|
||||
экранирования/рендера → деградация к существующему fallback (`f"task-{task_id}"` /
|
||||
пропуск строки), не исключение наружу.
|
||||
|
||||
## 4. Изменения API
|
||||
|
||||
Нет. HTTP-эндпоинты не добавляются/не меняются. (Внешний вызов — только исходящий
|
||||
`editMessageText`/`sendMessage` к Telegram Bot API; контракт вызова не меняется, меняется
|
||||
лишь безопасность `text`.)
|
||||
|
||||
## 5. Изменения схемы БД
|
||||
|
||||
Нет. Таблицы `tasks`/`agent_runs`/`tracker_messages` не затрагиваются; миграций нет.
|
||||
|
||||
## 6. Требования к новым/изменённым QG checks
|
||||
|
||||
Нет. `QG_CHECKS` / `check_*` / `STAGE_TRANSITIONS` / машинные вердикты не затрагиваются. Баг —
|
||||
в слое рендера уведомлений, вне Quality Gate.
|
||||
|
||||
## 7. Совместимость / регресс
|
||||
|
||||
- **Обратная совместимость:** изменение чисто в формировании строки текста карточки; данные
|
||||
БД, схема, режимы трекера (`bump`/`edit`), леджер сирот (ORCH-087), статусная модель
|
||||
(ORCH-066) — без изменений.
|
||||
- **Область раската:** все проекты на общем прод-инстансе (self-hosting) — фикс применяется к
|
||||
каждому новому рендеру сразу после деплоя; не требует миграции/бэкфилла.
|
||||
- **Kill-switch:** не требуется (исправление дефекта корректности, а не новая фича-ветка). Если
|
||||
архитектор выбирает переклассификацию parse-фейла в `update_task_tracker` (FR-4 опц.) —
|
||||
оценить целесообразность флага; по умолчанию изменение поведения минимально и безопасно.
|
||||
- **Обратимость:** изменение откатывается обычным revert PR (только `notifications.py` +
|
||||
тесты + CHANGELOG); прод-контейнер не требует ручных операций над данными.
|
||||
- **Артефакты pipeline:** обновляются `12-review.md` (reviewer), `13-test-report.md` (tester),
|
||||
`06-adr/ADR-001-*.md` (архитектор — выбор точки экранирования и стратегии FR-4),
|
||||
`CHANGELOG.md`. Машинные вердикты гейтов — без изменений.
|
||||
- **Self-hosting:** обязательна стадия `deploy-staging` (8501) перед прод-деплоем; прод
|
||||
`orchestrator` не рестартуется в рамках разработки.
|
||||
97
docs/work-items/ORCH-095/03-acceptance-criteria.md
Normal file
97
docs/work-items/ORCH-095/03-acceptance-criteria.md
Normal file
@@ -0,0 +1,97 @@
|
||||
---
|
||||
work_item: ORCH-095
|
||||
stage: analysis
|
||||
author_agent: analyst
|
||||
status: ready-for-review
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# 03 — Критерии приёмки (Acceptance Criteria): ORCH-095 — HTML-инъекция «<1м» в render_task_tracker
|
||||
|
||||
Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: analysis
|
||||
|
||||
Формат: каждый критерий имеет **PASS** (что должно быть истинно для приёмки) и **FAIL**
|
||||
(что считается провалом). Любой машинный/ручной reviewer проверяет их буквально по файлам
|
||||
репозитория.
|
||||
|
||||
---
|
||||
|
||||
## AC-1 — Стадия < 1 мин не ломает парсер Telegram
|
||||
|
||||
**Условие:** `render_task_tracker` для задачи, у которой хотя бы одна стадия длилась < 60 с,
|
||||
выдаёт текст, безопасный для `parse_mode=HTML` (нет неэкранированного `<` в данных длительности).
|
||||
- **PASS:** В выходном тексте подстрока длительности «меньше минуты» представлена как `<1м`
|
||||
(или переформулированный безопасный вид `~0м` / `< 1 мин` без сырого `<`); `editMessageText`
|
||||
с этим текстом не вернул бы `400 can't parse entities: Unsupported start tag "1м"`. Юнит-тест
|
||||
на `_fmt_minutes(30)` / `render_task_tracker(...)` подтверждает отсутствие сырого `<` от
|
||||
длительности.
|
||||
- **FAIL:** Текст содержит сырой `<1м` (или иной литерал `<`+нецифра) из данных длительности;
|
||||
тест на парсинг/наличие сырого `<` падает.
|
||||
|
||||
---
|
||||
|
||||
## AC-2 — Все динамические поля карточки HTML-безопасны (юнит)
|
||||
|
||||
**Условие:** Существует юнит-тест, проверяющий, что каждое подставляемое **данные-поле**
|
||||
`render_task_tracker` экранировано: длительность, токены, стоимость (`$`), заголовок с
|
||||
спецсимволами `< > &`, статус-лейбл, имя модели/эффорт.
|
||||
- **PASS:** Тест рендерит карточку с заголовком, содержащим `<`, `>`, `&` (напр.
|
||||
`"A <b>x</b> & <1"`), и стадией < 1 мин; ассертит, что эти спецсимволы из ДАННЫХ
|
||||
присутствуют в выводе только в экранированном виде (`<`/`>`/`&`) и НЕ как
|
||||
сырые теги; одновременно нет двойного экранирования (`&lt;`).
|
||||
- **FAIL:** Тест отсутствует, либо любое из перечисленных данных-полей попадает в текст без
|
||||
экранирования, либо обнаруживается двойное экранирование.
|
||||
|
||||
---
|
||||
|
||||
## AC-3 — Регресс намеренной разметки (ссылка-номер, форматирование)
|
||||
|
||||
**Условие:** После фикса намеренная HTML-разметка карточки продолжает рендериться валидной и
|
||||
кликабельной.
|
||||
- **PASS:** Кликабельный номер задачи (`<a href="…">ORCH-095</a>` от `plane_issue_link`)
|
||||
присутствует в выводе как валидный незаэкранированный `<a>`-тег; строки `🔗 PR #n`/`📦`
|
||||
(`_done_link`) и любое форматирование-обёртка рендерятся; существующие тесты
|
||||
`test_tracker_issue_link.py`/`test_notify_issue_links.py`/`test_telegram_tracker.py`
|
||||
зелёные. Двойного экранирования href/label нет.
|
||||
- **FAIL:** Номер задачи перестал быть кликабельным (`<a>` заэкранирован в `<a>`), либо
|
||||
любой регресс-тест разметки красный.
|
||||
|
||||
---
|
||||
|
||||
## AC-4 — Застрявшая карточка возобновляет обновления
|
||||
|
||||
**Условие:** Карточка, ранее застрявшая на `400 can't parse entities` (класс ORCH-093), после
|
||||
фикса снова обновляется.
|
||||
- **PASS:** На следующем переходе стадии текст рендера больше не содержит небезопасной
|
||||
подстроки → `editMessageText` проходит (`200`); ИЛИ (если выбрана стратегия FR-4-опц.)
|
||||
перманентный parse-фейл классифицируется как повод переотправить свежую карточку, и
|
||||
`update_task_tracker` отправляет новую. Поведение покрыто тестом (рендер валиден → edit-путь
|
||||
не возвращает `EDIT_FAILED` из-за parse-ошибки).
|
||||
- **FAIL:** После фикса карточка с прежним содержимым по-прежнему даёт `EDIT_FAILED` и не
|
||||
обновляется/не переотправляется; либо защита от дублей (ORCH-087) сломана — транзиентный
|
||||
фейл теперь плодит дубликаты карточек.
|
||||
|
||||
---
|
||||
|
||||
## AC-5 — never-raise, зелёный регресс, CHANGELOG
|
||||
|
||||
**Условие:** Контракт надёжности и гигиена изменения сохранены.
|
||||
- **PASS:** `render_task_tracker`/`update_task_tracker`/`edit_telegram` не выбрасывают
|
||||
исключение наружу при любом входе (включая «битый» заголовок/None); `pytest tests/ -q`
|
||||
полностью зелёный; в `CHANGELOG.md` есть запись о фиксе ORCH-095; `STAGE_TRANSITIONS`/
|
||||
`QG_CHECKS`/`check_*`/схема БД не изменены (diff их не трогает).
|
||||
- **FAIL:** Любой тест в `tests/` красный; обнаружено непойманное исключение в пути рендера;
|
||||
тронуты машина стадий/гейты/схема БД; нет записи в `CHANGELOG.md`.
|
||||
|
||||
---
|
||||
|
||||
## Сводная матрица AC ↔ FR/BR
|
||||
|
||||
| AC | Покрывает |
|
||||
|----|-----------|
|
||||
| AC-1 | BR-1, BR-3 / FR-1 |
|
||||
| AC-2 | BR-2 / FR-2 |
|
||||
| AC-3 | BR-4 / FR-3 |
|
||||
| AC-4 | BR-5 / FR-4 |
|
||||
| AC-5 | NFR-1, NFR-2 / FR-5 |
|
||||
95
docs/work-items/ORCH-095/04-test-plan.yaml
Normal file
95
docs/work-items/ORCH-095/04-test-plan.yaml
Normal file
@@ -0,0 +1,95 @@
|
||||
work_item: ORCH-095
|
||||
stage: analysis
|
||||
author_agent: analyst
|
||||
status: ready-for-review
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
title: "HTML-безопасность динамических полей render_task_tracker (фикс инъекции «<1м»)"
|
||||
framework: pytest
|
||||
scope: >
|
||||
Покрывается: HTML-безопасность всех подставляемых данных в render_task_tracker
|
||||
(длительности < 1 мин, токены/стоимость, имя модели/эффорт, статус-лейбл, заголовок со
|
||||
спецсимволами), сохранность намеренной разметки (<a href> номер задачи, _done_link),
|
||||
возобновление обновлений застрявшей карточки, never-raise. Вне покрытия: реальная сеть к
|
||||
Telegram Bot API (мокируется httpx), изменения STAGE_TRANSITIONS/QG_CHECKS/схемы БД (не
|
||||
трогаются).
|
||||
notes: >
|
||||
Тесты — изоляция от сети: httpx.post/get мокируются; БД — временная SQLite-фикстура с
|
||||
задачей и agent_runs (стадия < 60 с). Полный регресс pytest tests/ -q должен оставаться
|
||||
зелёным, включая существующие test_telegram_tracker.py / test_tracker_*.py /
|
||||
test_notifications_orphans.py / test_notify_issue_links.py. Регрессом считается: красный
|
||||
любой существующий тест трекера, заэкранированная намеренная разметка, двойное
|
||||
экранирование, непойманное исключение в пути рендера.
|
||||
|
||||
tests:
|
||||
- id: TC-01
|
||||
type: unit
|
||||
description: "_fmt_minutes для длительности < 60 с (напр. 30) не возвращает сырой '<1м': результат HTML-безопасен (<1м либо переформулированный '~0м'/'< 1 мин' без сырого '<')."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-02
|
||||
type: unit
|
||||
description: "_fmt_minutes для граничных входов (0, None, нечисловое, ровно 60, большое значение) — never-raise и HTML-безопасный вывод во всех ветках."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-03
|
||||
type: unit
|
||||
description: "render_task_tracker для задачи со стадией < 1 мин: в выходном тексте нет неэкранированного '<' из данных длительности; подстрока длительности безопасна для parse_mode=HTML."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-04
|
||||
type: unit
|
||||
description: "render_task_tracker с заголовком, содержащим спецсимволы '<', '>', '&' (напр. 'A <b>x</b> & <1'): спецсимволы данных присутствуют только экранированными (</>/&), не как сырые теги; двойного экранирования (&lt;) нет."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-05
|
||||
type: unit
|
||||
description: "Статус-лейбл (_card_status_label) и имя модели/эффорт, попадающие в текст карточки, экранированы (defence-in-depth): спецсимволы в них не ломают HTML."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-06
|
||||
type: unit
|
||||
description: "Метрики токенов/стоимости (fmt_tokens/fmt_cost) в карточке HTML-безопасны: '$' и числовой формат не порождают сырых тегов."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-07
|
||||
type: unit
|
||||
description: "Регресс намеренной разметки: кликабельный номер задачи (plane_issue_link -> <a href>) присутствует в выводе как валидный незаэкранированный <a>-тег; href/label не задвоены экранированием."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-08
|
||||
type: unit
|
||||
description: "Регресс _done_link: для завершённой задачи строка '🔗 PR #n · 📦 Внедрено' рендерится валидной (ссылочная разметка не экранирована)."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-09
|
||||
type: integration
|
||||
description: "update_task_tracker (edit-режим) с замоканным editMessageText: текст карточки со стадией < 1 мин принимается (мок ассертит отсутствие 'can't parse entities'-триггера, т.е. нет сырого '<1м' в payload text)."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-10
|
||||
type: integration
|
||||
description: "Возобновление застрявшей карточки (AC-4): после фикса валидный рендер проходит edit-путь без EDIT_FAILED из-за parse-ошибки; защита от дублей сохранена — транзиентный (network) фейл по-прежнему НЕ плодит новую карточку."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-11
|
||||
type: unit
|
||||
description: "never-raise: render_task_tracker на 'битых' входах (отсутствует задача, None-заголовок, нечисловые длительности) возвращает fallback-строку, не выбрасывает исключение."
|
||||
module: tests/test_tracker_html_escape.py
|
||||
expected: PASS
|
||||
|
||||
- id: TC-12
|
||||
type: integration
|
||||
description: "Полный регресс существующих тестов трекера (test_telegram_tracker.py, test_tracker_issue_link.py, test_tracker_status_line.py, test_notifications_orphans.py, test_notify_issue_links.py) остаётся зелёным после фикса."
|
||||
module: tests/test_telegram_tracker.py
|
||||
expected: PASS
|
||||
@@ -0,0 +1,209 @@
|
||||
---
|
||||
work_item: ORCH-095
|
||||
stage: architecture
|
||||
author_agent: architect
|
||||
status: accepted
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# ADR-001: HTML-безопасный рендер данных live-карточки трекера (устранение инъекции «<1м»)
|
||||
|
||||
Work Item: **ORCH-095** — HTML-инъекция `<1м` в `render_task_tracker` застывает live-карточку
|
||||
Стадия: **architecture**
|
||||
Сквозная регистрация: **N/A — локальное решение задачи.** Изменение целиком в слое рендера
|
||||
уведомлений (`src/notifications.py`); новой стадии/QG/компонента/смены БД нет, инварианты
|
||||
`STAGE_TRANSITIONS`/`QG_CHECKS`/схемы не затрагиваются → глобальный `adr-NNNN` не заводится
|
||||
(прецедент — ORCH-091, такой же indication-only фикс рендера, тоже без сквозного ADR).
|
||||
|
||||
## Статус
|
||||
Accepted
|
||||
|
||||
## Контекст
|
||||
|
||||
Live-карточка задачи (`src/notifications.py::render_task_tracker`) — основной канал видимости
|
||||
конвейера для оператора, инвариант «одна карточка на задачу» (ORCH-042/067/087). Карточка
|
||||
отправляется и редактируется с `parse_mode=HTML` (`send_telegram:58`, `edit_telegram:175`).
|
||||
|
||||
**Сверено по коду.** `_fmt_minutes(seconds)` (`notifications.py:280-290`) при `0 < seconds < 60`
|
||||
возвращает литерал `"<1м"`:
|
||||
|
||||
```python
|
||||
if seconds < 60:
|
||||
return "<1м"
|
||||
```
|
||||
|
||||
Эта подстрока интерполируется в HTML-текст карточки **без экранирования** (`_stage_line`:
|
||||
`dur = _fmt_minutes(dur_sum)` → строка `f"✅ {label:<13} {dur} · …"`; те же `_fmt_minutes` /
|
||||
`_capped_review_str` в строке BRD и в итоговой строке времени). Telegram трактует `<1м` как
|
||||
открывающий HTML-тег → `editMessageText` отвечает `400 Bad Request: can't parse entities:
|
||||
Unsupported start tag "1м"`. В `edit_telegram` неизвестный `400` классифицируется как
|
||||
`EDIT_FAILED` (`notifications.py:203`), а `update_task_tracker` по ветке `EDIT_FAILED` делает
|
||||
ранний `return` (анти-дубль ORCH-087) → **карточка застывает** (воспроизведено детерминированно
|
||||
09.06 на ORCH-093, `message_id 18854`).
|
||||
|
||||
**Корневой класс шире одного `<1м`.** Текст карточки — смесь (а) намеренной разметки-обёртки
|
||||
(`<a href>` номер задачи `num_html`, `link_for`, `_done_link`; заголовок уже экранирован как
|
||||
`esc_title`, `notifications.py:428`) и (б) подставляемых **данных**. Экранирована только
|
||||
категория-обёртка (href/label в `plane_issue_link` через `html.escape(..., quote=True)`) и
|
||||
заголовок. Прочие данные — длительности (`_fmt_minutes`/`_capped_review_str`), статус-лейбл
|
||||
(`_card_status_label` → `status_label`), имя модели (`short_model_name`), эффорт (`_run_effort`),
|
||||
токены/стоимость (`fmt_tokens`/`fmt_cost`) — вставляются сырыми. `<1м` — первый сработавший
|
||||
экземпляр класса «неэкранированные данные в HTML-тексте»; ТЗ требует закрыть класс, а не символ
|
||||
(BR-2/FR-2).
|
||||
|
||||
«Как есть» не годится: симптом плавающий (ловится только когда хотя бы одна стадия длилась
|
||||
< 60 с и её строка попадает в редактируемый текст), а отказ перманентный для конкретной карточки
|
||||
до конца жизни задачи — оператор слепнет.
|
||||
|
||||
## Решение
|
||||
|
||||
### Сводка
|
||||
|
||||
Локализуем HTML-безопасность в **границе рендера**: каждое подставляемое **данные-значение**
|
||||
экранируется `html.escape(...)` ровно один раз в точке интерполяции в `render_task_tracker`;
|
||||
функции-источники данных (`_fmt_minutes`, `short_model_name`, `_run_effort`, `fmt_tokens`,
|
||||
`fmt_cost`, `_card_status_label`) остаются **HTML-агностичными** (производят данные, не разметку).
|
||||
Намеренная разметка-обёртка (`num_html`, `link_for(...)`, `_done_link`, уже-экранированный
|
||||
`esc_title`) через экранирование **не** проходит. Литерал `<1м` в `_fmt_minutes` **сохраняется
|
||||
как есть**: будучи экранированным на границе (`<1м`), он рендерится оператору визуально
|
||||
идентично (`<1м`) → видимый формат не меняется, согласование формулировки не требуется.
|
||||
|
||||
### D1 — Точка внесения экранирования: граница рендера, не источник данных (⇒ FR-1, FR-2)
|
||||
|
||||
Экранирование делается на **потребителе** (внутри `render_task_tracker`/`_stage_line`), а не
|
||||
внутри функций-источников. Модель «слотов»: текст карточки собирается из слотов двух категорий —
|
||||
|
||||
- **Категория M (markup, НЕ экранировать):** `num_html` (`plane_issue_link`, внутри уже
|
||||
экранированы href+label), `link_for(...)` в строке «⏳ ждёт …», `_done_link(...)`
|
||||
(«🔗 PR #n · 📦 Внедрено»), `esc_title` (уже экранирован в строке 428).
|
||||
- **Категория D (data, экранировать ровно один раз):** `dur` (`_fmt_minutes`/`_capped_review_str`),
|
||||
`status_label` (`_card_status_label`), `model` (`short_model_name`), `effort` (`_run_effort`),
|
||||
`in_tok`/`out_tok` (`fmt_tokens`), `cost` (`fmt_cost`), а также числовые `attempt` и static-лейблы
|
||||
стадий (`_TRACKER_STAGES`/`_BRD_LABEL` — статичны и безопасны, но проходят через D ради
|
||||
единообразного инварианта).
|
||||
|
||||
Рекомендуемая реализация (необязательна к буквальному следованию — выбор формы за developer):
|
||||
завести тонкий модуль-локальный хелпер `def _esc(x): return html.escape(str(x))` (never-raise:
|
||||
на исключении `str()` → пустая строка/исходный fallback) и обернуть им каждый D-слот в момент
|
||||
присваивания, например `dur = _esc(_fmt_minutes(dur_sum))`, `model = _esc(short_model_name(...))`,
|
||||
`status_label = _esc(status_label)`. Источники данных НЕ трогаются (в т.ч. `src/usage.py` —
|
||||
`fmt_tokens`/`fmt_cost`/`short_model_name` остаются как есть; defence-in-depth делается на
|
||||
потребителе, как зафиксировано в ТЗ §2).
|
||||
|
||||
**Почему граница рендера, а не источник.** (1) Single-responsibility: `_fmt_minutes` и
|
||||
`short_model_name` используются и вне HTML-контекста (логи, потенциально иные потребители) —
|
||||
вшивать `<` в их вывод сделало бы данные «грязными» в не-HTML-контексте. (2) Инвариант FR-2
|
||||
формулируется и тестируется как свойство ОДНОЙ функции (`render_task_tracker`): «ни один символ
|
||||
`< > &` из данных не остаётся неэкранированным в выходе» — а не как разрозненные контракты пяти
|
||||
источников. (3) Экранирование на границе по построению исключает двойное экранирование: каждый
|
||||
D-слот экранируется в ровно одной точке; M-слоты не экранируются вовсе.
|
||||
|
||||
**Инвариант D1:** видимый оператору формат всех D-полей не меняется (escape `<1м`→`<1м`
|
||||
рендерится как `<1м`; `~Nм`, `Nм`, токены/стоимость/модель символов `< > &` не содержат →
|
||||
escape для них no-op).
|
||||
|
||||
### D2 — Сохранение `<1м` в источнике; формат-источник `_fmt_minutes` не меняется (⇒ FR-1, BR-3)
|
||||
|
||||
BR-3/FR-1 допускают два пути: (а) экранировать `<1м`, либо (б) переформулировать (`~0м` /
|
||||
`< 1 мин`). Выбираем **(а)**: `_fmt_minutes` продолжает возвращать `"<1м"`, безопасность даёт
|
||||
escape на границе (D1). Это минимизирует поверхность изменения (никаких правок числовой/строковой
|
||||
логики `_fmt_minutes`, `_capped_review_str`, тестов формата длительности) и сохраняет видимый
|
||||
оператору вид `<1м` без согласования новой формулировки. `_fmt_minutes` сохраняет never-raise
|
||||
(нечисловой/None → `0м`) без изменений.
|
||||
|
||||
### D3 — Defence-in-depth: экранируются ВСЕ D-поля, включая сейчас-безопасные (⇒ FR-2, BR-2)
|
||||
|
||||
Экранируются все поля категории D, в т.ч. сейчас гарантированно безопасные (`fmt_tokens`/
|
||||
`fmt_cost` дают только цифры/`.`/`k`/`M`/`$`; `short_model_name` — `^claude-…$`). Стоимость
|
||||
нулевая (escape безопасной строки — no-op), выгода — **структурный инвариант**: «каждый D-слот
|
||||
карточки экранирован», который защищает от регрессии при будущей смене формата любого источника
|
||||
(напр. если в имя модели/эффорта когда-нибудь попадёт пользовательский ввод). Тест AC-2 ассертит
|
||||
инвариант, а не отдельные поля.
|
||||
|
||||
### D4 — FR-4 (восстановление застрявших карточек): авто-recovery следующим рендером; парс-фейл НЕ переклассифицируется (⇒ BR-5, FR-4)
|
||||
|
||||
Механизм восстановления — **достаточное условие по умолчанию** из FR-4: после деплоя фикса на
|
||||
ближайшем переходе стадии `update_task_tracker` рендерит НОВЫЙ безопасный текст и вызывает
|
||||
`edit_telegram(mid, new_text)` → Telegram отвечает `200` → застрявшая карточка (класс ORCH-093)
|
||||
обновляется на месте. **Нового кода не требуется.**
|
||||
|
||||
Опциональную переклассификацию `can't parse entities` в `edit_telegram`/`update_task_tracker`
|
||||
(переотправка свежей карточки вместо `EDIT_FAILED`) **отвергаем**:
|
||||
|
||||
- **Не помогает.** Если текст всё ещё небезопасен, `send_telegram` упадёт на том же `400`
|
||||
идентично `editMessageText` (тот же `parse_mode=HTML`) и вернёт `None` → новой карточки нет.
|
||||
После фикса D1–D3 источник `can't parse entities` из НАШИХ данных структурно устранён, поэтому
|
||||
отдельная ветка восстановления лечит несуществующий после фикса случай.
|
||||
- **Риск.** Любое касание ветки `EDIT_FAILED`/леджера сирот рискует инвариантом ORCH-087
|
||||
(транзиентный фейл НЕ должен плодить карточки). Минимальная поверхность безопаснее.
|
||||
|
||||
`edit_telegram`, `update_task_tracker`, `send_telegram`, леджер `tracker_messages`, режимы
|
||||
`bump`/`edit` — **не трогаются**. Known-limitation (унаследовано ORCH-087): для карточки, у
|
||||
которой после фикса больше НЕ будет переходов стадии (задача завершилась до деплоя), повторного
|
||||
рендера не возникнет → карточка остаётся замёрзшей; Telegram-лимит 48ч делает её неперезаписываемой
|
||||
вне окна. BR-5 относится к карточкам в пределах окна с предстоящими переходами.
|
||||
|
||||
### D5 — Граница «данные vs обёртка»: M-слоты неприкосновенны, двойное экранирование запрещено (⇒ FR-3, BR-4)
|
||||
|
||||
`num_html` (`plane_issue_link`), `link_for(...)`, `_done_link(...)` и `esc_title` через `_esc`
|
||||
НЕ проходят — остаются валидным HTML, номер задачи кликабелен. Внутренности `plane_issue_link`
|
||||
(href `html.escape(url, quote=True)`, label `html.escape(work_item_id)`) уже экранированы — повторно
|
||||
их не экранируем (иначе `&lt;`, регресс AC-2/AC-3). Граница явная и тестируемая: D-слот → `_esc`;
|
||||
M-слот → as-is.
|
||||
|
||||
### D6 — Трассировка и инварианты соседних маркеров (⇒ NFR-2, NFR-3)
|
||||
|
||||
`render_task_tracker`/`_stage_line` несут маркеры ORCH-042/067/087/091. Изменение ORCH-095
|
||||
**аддитивно** к ним и обязано сохранить их инварианты: «одна карточка на задачу», леджер сирот и
|
||||
анти-дубль (ORCH-087), отражение откатов + суммирование метрик `_stage_line` (ORCH-091), строка
|
||||
Plane-статуса/кликабельный номер (ORCH-067). Поскольку ORCH-095 лишь оборачивает уже вычисленные
|
||||
D-значения в `_esc`, не меняя ни состава строк, ни порядка, ни логики подавления/суммирования —
|
||||
инварианты сохраняются по построению. Новые/изменённые строки помечаются маркером `ORCH-095`;
|
||||
блок остаётся читаемым (не вводим 3+ новых маркера в один блок → сводный сквозной ADR не требуется,
|
||||
TRACEABILITY анти-археология соблюдена).
|
||||
|
||||
## Альтернативы
|
||||
|
||||
- **Экранировать в источнике (`_fmt_minutes` возвращает `<1м`)** — отвергнуто: пачкает данные
|
||||
в не-HTML-контексте (логи), размазывает инвариант FR-2 по пяти функциям, усложняет защиту от
|
||||
двойного экранирования (D1).
|
||||
- **Переформулировать `<1м` → `~0м`/`< 1 мин`** — отвергнуто: меняет видимый оператору формат
|
||||
(требует согласования), трогает логику/тесты `_fmt_minutes`; escape на границе достигает того же
|
||||
при меньшей поверхности и нулевом визуальном изменении (D2).
|
||||
- **Переключить карточку на `parse_mode=None`/MarkdownV2** — отвергнуто (вне объёма BRD §6):
|
||||
сломает намеренную разметку (`<a href>` номер, `<b>`), MarkdownV2 требует экранирования ещё
|
||||
большего набора символов.
|
||||
- **Переклассификация `can't parse entities` → переотправка** — отвергнуто (D4): не помогает
|
||||
(send падает идентично), риск инварианту анти-дубля ORCH-087.
|
||||
|
||||
## Последствия
|
||||
|
||||
- **+** Класс «неэкранированные данные в HTML-тексте карточки» закрыт целиком (BR-2); `<1м` и
|
||||
любые будущие `< > &` из данных безопасны; карточка со стадией < 1 мин редактируется (`200`).
|
||||
- **+** Структурный defence-in-depth инвариант («каждый D-слот экранирован»), тестируемый одним
|
||||
свойством `render_task_tracker` (AC-2), устойчив к будущим сменам формата источников.
|
||||
- **+** Видимый формат карточки и намеренная разметка (кликабельный номер, `_done_link`) без
|
||||
изменений (BR-3/BR-4); никаких миграций/правок схемы/гейтов (NFR-3/NFR-4).
|
||||
- **+** Застрявшие (в окне) карточки авто-восстанавливаются следующим рендером без нового кода
|
||||
(BR-5).
|
||||
- **−** Точечная дисциплина «D-слот → `_esc`, M-слот → as-is» вносит точку для будущих ошибок
|
||||
(можно забыть обернуть новый D-слот или по ошибке обернуть M-слот → двойное экранирование).
|
||||
Митигейшн: тест-инвариант AC-2 (нет сырого `< > &` из данных И нет `&lt;`) ловит обе
|
||||
ошибки; явный реестр M-слотов в D5.
|
||||
- **−** Карточки задач, завершившихся до деплоя фикса, не восстанавливаются (нет будущего
|
||||
рендера) — known-limitation, унаследовано ORCH-087/Telegram-48ч; вне управляемого.
|
||||
- **Откат:** обычный revert PR (только `src/notifications.py` + тесты + `CHANGELOG.md` +
|
||||
doc-правки); прод-контейнер `orchestrator` не требует ручных операций над данными/БД.
|
||||
|
||||
## Ссылки
|
||||
- BRD: `docs/work-items/ORCH-095/01-brd.md`
|
||||
- TRZ: `docs/work-items/ORCH-095/02-trz.md`
|
||||
- Acceptance: `docs/work-items/ORCH-095/03-acceptance-criteria.md`
|
||||
- Tech-risks: `docs/work-items/ORCH-095/10-tech-risks.md`
|
||||
- Сверено по коду: `src/notifications.py` (`_fmt_minutes:280-290`, `_capped_review_str:315-336`,
|
||||
`render_task_tracker:355-610`, `_stage_line:467-507`, `_card_status_label:1173-1186`,
|
||||
`plane_issue_link:932-949`, `_done_link:613-647`, `link_for:952-984`, `edit_telegram:157-207`,
|
||||
`update_task_tracker:650-746`, `send_telegram:42-71`, `esc_title:428`)
|
||||
- Инварианты соседей: ORCH-042/067 (карточка/номер), ORCH-087 (леджер сирот/анти-дубль),
|
||||
ORCH-091 (откаты/суммирование `_stage_line`) — `docs/architecture/internals.md` §7
|
||||
37
docs/work-items/ORCH-095/10-tech-risks.md
Normal file
37
docs/work-items/ORCH-095/10-tech-risks.md
Normal file
@@ -0,0 +1,37 @@
|
||||
---
|
||||
work_item: ORCH-095
|
||||
stage: architecture
|
||||
author_agent: architect
|
||||
status: accepted
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
---
|
||||
|
||||
# 10 — Технические риски: ORCH-095 — HTML-безопасность данных live-карточки
|
||||
|
||||
Work Item: **ORCH-095** · Repo: **orchestrator** · Стадия: architecture
|
||||
|
||||
> Информационный (гейтом не парсится). Перечисляет риски реализации и их митигейшн.
|
||||
|
||||
## Реестр рисков
|
||||
|
||||
| ID | Риск | Вер. | Влия. | Митигейшн |
|
||||
|----|------|------|-------|-----------|
|
||||
| TR-1 | **Двойное экранирование** уже-экранированных полей (`esc_title`, href/label внутри `plane_issue_link`) → `&lt;` в выводе, визуальный мусор / регресс AC-2 | Сред. | Сред. | D1/D5: явный реестр M-слотов (markup) — через `_esc` НЕ проходят; `esc_title` остаётся единственной точкой escape заголовка; тест AC-2 ассертит отсутствие `&lt;` |
|
||||
| TR-2 | **Случайное экранирование разметки-обёртки** (`num_html`/`link_for`/`_done_link`) → `<a>` превращается в `<a>`, номер задачи перестаёт быть кликабельным (регресс BR-4/AC-3) | Низ. | Выс. | D5: M-слоты неприкосновенны; регресс-тесты `test_tracker_issue_link.py`/`test_notify_issue_links.py`/`test_telegram_tracker.py` зелёные; AC-3 проверяет наличие валидного `<a href>` в выводе |
|
||||
| TR-3 | **Пропущен новый/существующий D-слот** (забыли обернуть `_esc`) → инъекция возвращается на другом поле | Низ. | Сред. | D3 defence-in-depth (обернуть ВСЕ D-поля разом); тест-инвариант AC-2 рендерит карточку с `< > &` в данных и ассертит отсутствие сырых спецсимволов из данных в выводе (свойство `render_task_tracker`, не пер-поле) |
|
||||
| TR-4 | **Регресс never-raise**: `_esc(str(x))` на «битом» входе (объект с падающим `__str__`) бросает исключение в пути рендера (нарушение NFR-1) | Низ. | Сред. | FR-5: `_esc` сам never-raise (try/except → fallback-строка); путь `render_task_tracker`/`update_task_tracker` уже обёрнут `try/except` (строки 654/745); тест AC-5 с «битым» входом |
|
||||
| TR-5 | **Застрявшая карточка не восстановилась** (задача завершилась до деплоя → нет будущего рендера) | Сред. | Низ. | Принятая known-limitation (D4): авто-recovery работает только при предстоящем переходе стадии; вне окна — Telegram-48ч (унаследовано ORCH-087); BR-5 ограничен карточками в окне |
|
||||
| TR-6 | **Скрытая регрессия инвариантов соседних маркеров** (ORCH-087 анти-дубль, ORCH-091 суммирование `_stage_line`) при правке тела `_stage_line`/`render_task_tracker` | Низ. | Выс. | D6: изменение аддитивно (лишь оборачивает уже вычисленные значения в `_esc`), не меняет состав/порядок строк, логику подавления откатов и суммирования; полный регресс `pytest tests/ -q` зелёный (NFR-2) |
|
||||
| TR-7 | **Self-hosting**: фикс деплоится на общий прод-инстанс (затронуты и enduro-trails) | Низ. | Сред. | NFR-3: изменение только слоя рендера; `STAGE_TRANSITIONS`/`QG_CHECKS`/схема БД не тронуты; обязательная страховка `deploy-staging` (8501) перед прод-деплоем; прод `orchestrator` не рестартится в рамках разработки |
|
||||
|
||||
## Сводный вывод
|
||||
|
||||
Доминирующий класс рисков — **регресс рендера** (двойное экранирование / случайное экранирование
|
||||
разметки / пропущенный D-слот), полностью покрываемый тест-инвариантом AC-2 + существующими
|
||||
регресс-тестами трекера (AC-3/AC-5). Изменение **локализовано** в `src/notifications.py` (слой
|
||||
рендера уведомлений), аддитивно к маркерам ORCH-042/067/087/091, не затрагивает машину стадий,
|
||||
Quality Gates, схему БД, транспортные примитивы и режимы трекера. Остаточный риск для
|
||||
прод-конвейера (self-hosting) — **низкий**: контракт never-raise сохранён, откат — обычный revert
|
||||
PR без операций над данными. Эскалация `arch:major-change` **не требуется**; возврат в анализ
|
||||
**не требуется** (ТЗ реализуемо без нарушения архитектурных принципов).
|
||||
81
docs/work-items/ORCH-095/12-review.md
Normal file
81
docs/work-items/ORCH-095/12-review.md
Normal file
@@ -0,0 +1,81 @@
|
||||
---
|
||||
verdict: APPROVED
|
||||
work_item: ORCH-095
|
||||
stage: review
|
||||
author_agent: reviewer
|
||||
status: approved
|
||||
created_at: 2026-06-10
|
||||
model_used: claude-opus-4-8
|
||||
type: review
|
||||
work_item_id: ORCH-095
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-095
|
||||
|
||||
## Summary
|
||||
|
||||
Фикс HTML-инъекции `<1м` в live-карточке трекера. Точечное, аддитивное, never-raise изменение
|
||||
в индикативном слое (`src/notifications.py`): новый модуль-локальный хелпер `_esc(x) =
|
||||
html.escape(str(x))` оборачивает каждый **data**-слот (`dur`/`_fmt_minutes`/`_capped_review_str`,
|
||||
`status_label`, `model`, `effort`, токены/стоимость) ровно один раз на границе рендера
|
||||
(`render_task_tracker`/`_stage_line`); **markup**-слоты (`num_html`/`link_for`/`_done_link`/
|
||||
уже-экранированный `esc_title`) не трогаются. Источники (`_fmt_minutes`, `src/usage.py`) остаются
|
||||
HTML-агностичными.
|
||||
|
||||
Проверены все четыре оси. Реализация соответствует ТЗ (FR-1…FR-5) и ADR-001 (D1…D6) буквально;
|
||||
все 5 AC выполнены. `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / схема БД / транспорт нотификаций
|
||||
— не тронуты (`git diff` пуст по `src/stages.py`, `src/qg/`, `src/stage_engine.py`, `src/db.py`).
|
||||
Полный регресс `pytest tests/ -q` зелёный (**1437 passed**), новый `tests/test_tracker_html_escape.py`
|
||||
(TC-01…TC-11) — зелёный.
|
||||
|
||||
**Соответствие осям:**
|
||||
1. **ТЗ / AC** — FR-1/AC-1 (`<1м`→`<1м` на границе, источник не меняется), FR-2/AC-2 (все
|
||||
D-слоты экранированы — сверено по коду стр. 471/517-523/529/594/607/614-615/620-621/629),
|
||||
FR-3/AC-3 (M-слоты не экранированы, двойного экранирования нет), FR-4/AC-4 (авто-восстановление
|
||||
следующим рендером, без рискованной переклассификации `EDIT_FAILED` — корректно, защищает
|
||||
инвариант ORCH-087), FR-5/AC-5 (never-raise + зелёный регресс + CHANGELOG). ✓
|
||||
2. **ADR + трассировка** — реализация 1:1 с ADR-001 (escape на границе рендера, не в источнике;
|
||||
M-слоты неприкосновенны). Блоки с маркерами ORCH-042/067/087/091 правлены аддитивно: код лишь
|
||||
оборачивает уже вычисленные D-значения в `_esc`, не меняя состав строк/порядок/логику подавления
|
||||
и суммирования — инварианты сохранены по построению. Сквозной `adr-NNNN` обоснованно не заведён
|
||||
(локальный indication-only фикс). ✓
|
||||
3. **Качество кода** — `_esc` с docstring и never-raise; тесты содержательные (11 TC покрывают
|
||||
каждый AC, включая регресс кликабельного `<a href>`-номера, `_done_link` и анти-дубль ORCH-087
|
||||
на транзиентном фейле). ✓
|
||||
4. **Документация** — обновлены в том же PR: `CHANGELOG.md`, `docs/architecture/README.md`
|
||||
(блок Notifications/Live-tracker), `docs/architecture/internals.md` §7, ADR-001. ✓
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- Нет.
|
||||
|
||||
### P1 — Must fix
|
||||
- Нет.
|
||||
|
||||
### P2 — Should fix
|
||||
- Нет.
|
||||
|
||||
### P3 — Nice-to-have
|
||||
- [ ] `attempt` (`f"… попытка {attempt} …"`, ~стр. 572) и статичные лейблы стадий
|
||||
(`_TRACKER_STAGES`/`_BRD_LABEL`) не проходят через `_esc`. ADR-001 D1 упоминает их в категории D
|
||||
«ради единообразного инварианта», но `attempt` — всегда `int` (`len(agent_runs)`), а лейблы —
|
||||
статичные константы → фактической поверхности инъекции нет, расхождение безвредно. Не блокирует;
|
||||
можно унифицировать при будущем касании блока (оставляю на усмотрение, не требую правки).
|
||||
|
||||
## Документация
|
||||
|
||||
**Обновлена полностью в том же PR — требование правила 6 (CLAUDE.md) выполнено:**
|
||||
- `CHANGELOG.md` — детальная запись ORCH-095 (механизм бага, D1–D5, восстановление, трассировка, тесты).
|
||||
- `docs/architecture/README.md` — компонент «Notifications / Live-tracker» дополнен абзацем ORCH-095
|
||||
(data/markup-слоты, инвариант экранирования на границе, ссылка на ADR).
|
||||
- `docs/architecture/internals.md` §7 — новая подсекция «HTML-безопасность данных карточки (ORCH-095)».
|
||||
- `docs/work-items/ORCH-095/06-adr/ADR-001-html-safe-card-data-render.md` — архитектурное обоснование
|
||||
(выбор точки экранирования, альтернативы, последствия).
|
||||
|
||||
Пункт `README.md` «Известные ограничения» данным фиксом не закрывается (баг корректности, не числился
|
||||
в витрине ограничений) → обновление обзорной витрины (ORCH-079) не требуется.
|
||||
|
||||
**Вывод:** `src/` изменён — документация обновлена синхронно. P0 «документация не обновлена» не
|
||||
применяется.
|
||||
92
docs/work-items/ORCH-095/13-test-report.md
Normal file
92
docs/work-items/ORCH-095/13-test-report.md
Normal file
@@ -0,0 +1,92 @@
|
||||
---
|
||||
result: PASS # PASS | FAIL — машинный вердикт, UPPERCASE
|
||||
work_item: ORCH-095
|
||||
stage: testing
|
||||
author_agent: tester
|
||||
status: pass
|
||||
created_at: 2026-06-10
|
||||
model_used: claude-opus-4-8
|
||||
type: test-report
|
||||
work_item_id: ORCH-095
|
||||
---
|
||||
|
||||
# Test Report — ORCH-095
|
||||
|
||||
Фикс HTML-инъекции «<1м» в live-карточке трекера (`render_task_tracker`). Прогон полного
|
||||
регресса + профильной сюиты, smoke read-only API. Review-вердикт — `APPROVED` (12-review.md).
|
||||
|
||||
## Окружение
|
||||
- Python: 3.12.13
|
||||
- pytest: 8.3.3
|
||||
- Дата: 2026-06-10
|
||||
- Worktree: `/repos/_wt/orchestrator/feature_ORCH-095-bug-html-1-render-task-tracker`
|
||||
(ветка `feature/ORCH-095-bug-html-1-render-task-tracker` — код именно этой задачи, не общий чекаут)
|
||||
|
||||
## Smoke API (read-only, прод-контейнер не трогается)
|
||||
- `GET /health` → `{"status":"ok","service":"orchestrator"}` ✓
|
||||
- `GET /status` → активная задача ORCH-095 (id=80) на стадии `testing`, agent_running=null ✓
|
||||
- `GET /queue` → блок `serial_gate` **присутствует** (ORCH-088): `enabled=true`, репо
|
||||
`orchestrator` — active_task ORCH-095 `testing`, `frozen=false`, waiting пуст; блок
|
||||
`auto_labels` **присутствует** (ORCH-089). Регресса смока нет. ✓
|
||||
|
||||
## Результаты
|
||||
|
||||
### Полный регресс
|
||||
`cd <worktree> && pytest tests/ -v --tb=short` → **1437 passed, 1 warning in 46.89s**.
|
||||
Единственное предупреждение — PydanticDeprecatedSince20 (унаследованное, не относится к задаче).
|
||||
|
||||
### Профильная сюита (ORCH-095)
|
||||
`pytest tests/test_tracker_html_escape.py -v` → **24 passed** (новый файл, TC-01…TC-11).
|
||||
|
||||
### Регресс существующих тестов трекера (TC-12)
|
||||
`pytest tests/test_telegram_tracker.py tests/test_tracker_issue_link.py
|
||||
tests/test_tracker_status_line.py tests/test_notifications_orphans.py
|
||||
tests/test_notify_issue_links.py -q` → **91 passed**.
|
||||
|
||||
### Сопоставление с тест-планом (04-test-plan.yaml)
|
||||
|
||||
| TC ID | Описание | Тест-функция | Результат |
|
||||
|-------|----------|--------------|-----------|
|
||||
| TC-01 | `_fmt_minutes(<60с)` → HTML-безопасно, без сырого `<1м` | `test_tc01_sub_minute_duration_escaped_at_boundary` | PASS |
|
||||
| TC-02 | `_fmt_minutes` граничные входы (0/None/нечисло/60/большое/-5/59/61) — never-raise + безопасно | `test_tc02_fmt_minutes_never_raise_and_safe[*]` (9 кейсов) | PASS |
|
||||
| TC-03 | `render_task_tracker` со стадией < 1 мин — нет неэкранированного `<` из длительности | `test_tc03_render_sub_minute_stage_is_safe` | PASS |
|
||||
| TC-04 | Заголовок со спецсимволами `< > &` — только экранированно, без двойного экранирования | `test_tc04_title_special_chars_escaped_no_double` | PASS |
|
||||
| TC-05 | Статус-лейбл / имя модели / эффорт экранированы (defence-in-depth) | `test_tc05_status_label_escaped`, `test_tc05_model_escaped`, `test_tc05_effort_escaped` | PASS |
|
||||
| TC-06 | Токены/стоимость (`$`, числа) HTML-безопасны | `test_tc06_token_cost_metrics_safe` | PASS |
|
||||
| TC-07 | Регресс намеренной разметки: `<a href>` номер задачи остаётся кликабельным, не задвоен | `test_tc07_issue_number_stays_clickable` | PASS |
|
||||
| TC-08 | Регресс `_done_link`: строка `🔗 PR #n · 📦 Внедрено` валидна, не экранирована | `test_tc08_done_link_markup_preserved` | PASS |
|
||||
| TC-09 | `update_task_tracker` (edit) — payload text не содержит сырого `<1м`-триггера | `test_tc09_edit_payload_is_parse_safe` | PASS |
|
||||
| TC-10 | Возобновление застрявшей карточки + анти-дубль ORCH-087 на транзиентном фейле | `test_tc10_valid_render_edits_in_place_no_new_card`, `test_tc10_transient_fail_does_not_duplicate` | PASS |
|
||||
| TC-11 | never-raise на битых входах (нет задачи / None-заголовок / битые timestamps / `_esc`) | `test_tc11_never_raise_missing_task`, `test_tc11_never_raise_none_title_and_bad_timestamps`, `test_tc11_esc_never_raises` | PASS |
|
||||
| TC-12 | Полный регресс существующих тестов трекера остаётся зелёным | suite (91 passed) + полный регресс (1437 passed) | PASS |
|
||||
|
||||
**Все 12 TC выполнены и сопоставлены.**
|
||||
|
||||
### Сопоставление с критериями приёмки (03-acceptance-criteria.md)
|
||||
|
||||
| AC | Содержание | Покрытие | Результат |
|
||||
|----|------------|----------|-----------|
|
||||
| AC-1 | Стадия < 1 мин не ломает парсер Telegram (`<1м`) | TC-01, TC-03, TC-09 | PASS |
|
||||
| AC-2 | Все динамические поля HTML-безопасны, без двойного экранирования | TC-02, TC-04, TC-05, TC-06 | PASS |
|
||||
| AC-3 | Регресс намеренной разметки (`<a href>` номер, `_done_link`, форматирование) | TC-07, TC-08, TC-12 | PASS |
|
||||
| AC-4 | Застрявшая карточка возобновляет обновления; анти-дубль ORCH-087 цел | TC-10 | PASS |
|
||||
| AC-5 | never-raise, зелёный регресс, CHANGELOG, машина стадий/гейты/схема БД не тронуты | TC-11, TC-12, полный регресс 1437 passed | PASS |
|
||||
|
||||
## Вывод pytest
|
||||
```
|
||||
======================= 1437 passed, 1 warning in 46.89s =======================
|
||||
```
|
||||
Профильная сюита:
|
||||
```
|
||||
======================== 24 passed, 1 warning in 1.31s =========================
|
||||
```
|
||||
Регресс трекера (TC-12):
|
||||
```
|
||||
91 passed, 1 warning in 4.32s
|
||||
```
|
||||
|
||||
## Итог
|
||||
PASS — полный регресс зелёный (1437 passed), профильная сюита ORCH-095 зелёная (24 passed),
|
||||
каждый TC из тест-плана выполнен и сопоставлен с критериями приёмки, smoke API read-only
|
||||
(`/health`, `/status`, `/queue` с блоками `serial_gate` + `auto_labels`) без регресса.
|
||||
Обоснованных FAIL/смок-сбоев нет → `result: PASS` → задача переходит на `deploy-staging`.
|
||||
12
docs/work-items/ORCH-095/14-deploy-log.md
Normal file
12
docs/work-items/ORCH-095/14-deploy-log.md
Normal file
@@ -0,0 +1,12 @@
|
||||
---
|
||||
deploy_status: SUCCESS
|
||||
work_item: ORCH-095
|
||||
hook_exit_code: 0
|
||||
deployed_by: deploy-finalizer
|
||||
---
|
||||
|
||||
# Deploy log — ORCH-036 executable self-deploy
|
||||
|
||||
Прод-деплой завершён хост-хуком с exit-code `0` -> `deploy_status: SUCCESS`.
|
||||
|
||||
Вердикт зафиксирован детерминированным finalizer'ом (Фаза C), не LLM.
|
||||
@@ -290,6 +290,27 @@ def _fmt_minutes(seconds) -> str:
|
||||
return f"{seconds // 60}\u043c"
|
||||
|
||||
|
||||
def _esc(x) -> str:
|
||||
"""ORCH-095: escape a DATA value for the parse_mode=HTML card text (never-raise).
|
||||
|
||||
Every dynamic *data* value interpolated into ``render_task_tracker``'s HTML text
|
||||
(durations, status label, model, effort, token/cost metrics) is wrapped here
|
||||
exactly once at the render boundary (ADR-001, category D). This closes the class
|
||||
"unescaped data in HTML text": a literal like ``<1м`` from ``_fmt_minutes`` (or any
|
||||
future ``< > &`` from a data source) can no longer be parsed by Telegram as an
|
||||
opening tag (``400 can't parse entities`` -> EDIT_FAILED -> frozen card, ORCH-093).
|
||||
|
||||
Intentional markup slots (``num_html``/``link_for``/``_done_link``/already-escaped
|
||||
``esc_title`` — category M) are NOT passed through ``_esc`` so they stay valid,
|
||||
clickable HTML and are never double-escaped. On any error ``str()``/escape degrades
|
||||
to '' rather than raising (FR-5 never-raise).
|
||||
"""
|
||||
try:
|
||||
return html.escape(str(x))
|
||||
except Exception:
|
||||
return ""
|
||||
|
||||
|
||||
def _parse_sql_ts(ts):
|
||||
"""Parse a SQLite 'YYYY-MM-DD HH:MM:SS' UTC timestamp -> aware datetime/None."""
|
||||
if not ts:
|
||||
@@ -445,7 +466,9 @@ def render_task_tracker(task_id: int) -> str:
|
||||
)
|
||||
except Exception:
|
||||
status_label = _DEFAULT_STATUS_LABEL
|
||||
status_line = f"\U0001f4cd {status_label}"
|
||||
# ORCH-095 (ADR-001 D3): status label is a DATA slot (offline core + live
|
||||
# overlay) -> escaped at interpolation; intentional markup is never built here.
|
||||
status_line = f"\U0001f4cd {_esc(status_label)}"
|
||||
lines = [header, status_line, bar]
|
||||
|
||||
# ORCH-026 (B-4): waiting-line for a task blocked by an unfinished declared
|
||||
@@ -487,19 +510,23 @@ def render_task_tracker(task_id: int) -> str:
|
||||
d = _duration_seconds(run["started_at"], run["finished_at"])
|
||||
if d is not None:
|
||||
dur_sum += d
|
||||
in_tok = fmt_tokens(in_sum)
|
||||
out_tok = fmt_tokens(out_sum)
|
||||
cost = fmt_cost(cost_sum)
|
||||
dur = _fmt_minutes(dur_sum)
|
||||
# ORCH-095 (ADR-001 D1/D3): every interpolated DATA value (category D) is
|
||||
# escaped here at the render boundary so a literal like '<1м' from
|
||||
# _fmt_minutes can no longer break parse_mode=HTML; defence-in-depth for the
|
||||
# token/cost/model/effort fields too (currently safe, structurally guarded).
|
||||
in_tok = _esc(fmt_tokens(in_sum))
|
||||
out_tok = _esc(fmt_tokens(out_sum))
|
||||
cost = _esc(fmt_cost(cost_sum))
|
||||
dur = _esc(_fmt_minutes(dur_sum))
|
||||
# Model/effort/"\u043f\u043e\u043f\u044b\u0442\u043a\u0430 N" come from the LAST run (agent_runs are id ASC).
|
||||
last = stage_runs[-1] if stage_runs else None
|
||||
model = short_model_name(last["model"]) if last is not None else ""
|
||||
model = _esc(short_model_name(last["model"])) if last is not None else ""
|
||||
model_suffix = f" \u00b7 {model}" if model else ""
|
||||
# ORCH-087 (BR-EFF): render the resolved --effort next to the model
|
||||
# ("\u00b7 opus-4-8 \u00b7 xhigh"). Stamped at launch in agent_runs.effort; empty /
|
||||
# missing -> suffix omitted (like the model suffix). Historical rows with
|
||||
# NULL effort fall back to the config-resolved effort for the agent.
|
||||
effort = _run_effort(last) if last is not None else ""
|
||||
effort = _esc(_run_effort(last)) if last is not None else ""
|
||||
effort_suffix = f" \u00b7 {effort}" if effort else ""
|
||||
return (
|
||||
f"\u2705 {label:<13} {dur} \u00b7 "
|
||||
@@ -564,7 +591,7 @@ def render_task_tracker(task_id: int) -> str:
|
||||
if review_seconds is not None:
|
||||
# ORCH-042 (BR-10): approve-gate passed -> \u2705 (was \u23f8\ufe0f). The
|
||||
# still-waiting branch below keeps \u23f8\ufe0f + \u23f3 unchanged.
|
||||
dur = _fmt_minutes(review_seconds)
|
||||
dur = _esc(_fmt_minutes(review_seconds)) # ORCH-095: D-slot
|
||||
lines.append(
|
||||
f"\u2705 {brd_label} {dur} \u00b7 \u0442\u0432\u043e\u0451 \u0432\u0440\u0435\u043c\u044f"
|
||||
)
|
||||
@@ -577,21 +604,21 @@ def render_task_tracker(task_id: int) -> str:
|
||||
waited = int(
|
||||
(datetime.now(timezone.utc) - start_dt).total_seconds()
|
||||
)
|
||||
dur = _fmt_minutes(waited) if waited is not None else "\u2026"
|
||||
dur = _esc(_fmt_minutes(waited)) if waited is not None else "\u2026" # ORCH-095: D-slot
|
||||
lines.append(
|
||||
f"\u23f8\ufe0f {brd_label} {dur} \u00b7 \u0442\u0432\u043e\u0451 \u0432\u0440\u0435\u043c\u044f \u23f3"
|
||||
)
|
||||
|
||||
lines.append(bar)
|
||||
lines.append(
|
||||
f"\U0001f4b0 {fmt_tokens(total_in)}\u2193 / {fmt_tokens(total_out)}\u2191 \u00b7 "
|
||||
f"{fmt_cost(total_cost)}"
|
||||
f"\U0001f4b0 {_esc(fmt_tokens(total_in))}\u2193 / {_esc(fmt_tokens(total_out))}\u2191 \u00b7 "
|
||||
f"{_esc(fmt_cost(total_cost))}"
|
||||
)
|
||||
|
||||
if done:
|
||||
wall = _duration_seconds(task["created_at"], task["updated_at"])
|
||||
wall_str = _fmt_minutes(wall) if wall is not None else "?"
|
||||
review_str = _capped_review_str(review_seconds)
|
||||
wall_str = _esc(_fmt_minutes(wall)) if wall is not None else "?" # ORCH-095: D-slot
|
||||
review_str = _esc(_capped_review_str(review_seconds)) # ORCH-095: D-slot
|
||||
# ORCH-087 (BR-G5): three INDEPENDENT, explicitly-labelled metrics. None is
|
||||
# presented as the sum of the others \u2014 queue/wait pauses are not logged, so
|
||||
# wall != agents + review; the old "\u0412\u0441\u0435\u0433\u043e {wall}" read like a (wrong) sum.
|
||||
@@ -599,7 +626,7 @@ def render_task_tracker(task_id: int) -> str:
|
||||
# \u0442\u0432\u043e\u0451 = human BRD-review, capped to drop anomalous stalls (T-2)
|
||||
# \u043e\u0431\u0449\u0435\u0435 \u0441 \u043e\u0436\u0438\u0434\u0430\u043d\u0438\u0435\u043c = wall-clock incl. queue/wait, NOT work time (T-3)
|
||||
lines.append(
|
||||
f"\u23f1\ufe0f \u0410\u0433\u0435\u043d\u0442\u044b {_fmt_minutes(agent_seconds)} \u00b7 "
|
||||
f"\u23f1\ufe0f \u0410\u0433\u0435\u043d\u0442\u044b {_esc(_fmt_minutes(agent_seconds))} \u00b7 "
|
||||
f"\u0442\u0432\u043e\u0451 {review_str} \u00b7 "
|
||||
f"\u043e\u0431\u0449\u0435\u0435 \u0441 \u043e\u0436\u0438\u0434\u0430\u043d\u0438\u0435\u043c {wall_str}"
|
||||
)
|
||||
|
||||
358
tests/test_tracker_html_escape.py
Normal file
358
tests/test_tracker_html_escape.py
Normal file
@@ -0,0 +1,358 @@
|
||||
"""ORCH-095 — HTML-safety of dynamic data fields in render_task_tracker.
|
||||
|
||||
The live card text is sent/edited with parse_mode=HTML. It is assembled from
|
||||
two kinds of slots:
|
||||
|
||||
* category M (intentional markup, NEVER escaped): the clickable issue number
|
||||
(plane_issue_link -> <a href>), the ⏳-waiting links (link_for), the done
|
||||
line (_done_link), and the already-escaped title (esc_title);
|
||||
* category D (data, escaped EXACTLY once at the render boundary): durations
|
||||
(_fmt_minutes / _capped_review_str), the status label, model, effort, and
|
||||
the token/cost metrics.
|
||||
|
||||
The bug (ORCH-093 incident): _fmt_minutes returns the literal "<1м" for a
|
||||
sub-minute stage; interpolated raw into HTML text Telegram parsed "<1м" as an
|
||||
opening tag -> 400 can't parse entities -> EDIT_FAILED -> the card froze. ADR-001
|
||||
closes the whole class by escaping every D-slot at the boundary (helper N._esc)
|
||||
while keeping the M-slots intact (so the number stays clickable, no double-escape).
|
||||
|
||||
These tests assert: sub-minute durations are safe (TC-01/02/03), all data fields
|
||||
escape special chars without double-escaping (TC-04/05/06), markup survives
|
||||
(TC-07/08), the edit payload is parse-safe and the anti-duplicate invariant
|
||||
(ORCH-087) holds (TC-09/10), and the render path never raises (TC-11).
|
||||
|
||||
Network is isolated (no live overlay HTTP); the DB is a temp SQLite.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token")
|
||||
os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token")
|
||||
|
||||
_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_html_escape.db")
|
||||
os.environ["ORCH_DB_PATH"] = _test_db
|
||||
|
||||
from types import SimpleNamespace # noqa: E402
|
||||
from unittest.mock import MagicMock # noqa: E402
|
||||
|
||||
import pytest # noqa: E402
|
||||
|
||||
import src.db as db_module # noqa: E402
|
||||
import src.projects as projects_mod # noqa: E402
|
||||
from src.db import init_db, get_db # noqa: E402
|
||||
from src import notifications as N # noqa: E402
|
||||
|
||||
# orchestrator repo -> default project registry uuid (src/projects.py).
|
||||
_ORCH_PROJECT_ID = "8da6aa25-a60e-44d6-a1e2-d8ae59aa7d6a"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def setup_db(monkeypatch):
|
||||
monkeypatch.setattr(db_module.settings, "db_path", _test_db, raising=False)
|
||||
if os.path.exists(_test_db):
|
||||
os.unlink(_test_db)
|
||||
init_db()
|
||||
# Keep the render path fully offline (no live overlay HTTP).
|
||||
monkeypatch.setattr(N._get_settings(), "tracker_live_status", False,
|
||||
raising=False)
|
||||
monkeypatch.setattr(
|
||||
projects_mod, "get_project_by_repo",
|
||||
lambda repo: (SimpleNamespace(plane_project_id=_ORCH_PROJECT_ID)
|
||||
if repo == "orchestrator" else None),
|
||||
)
|
||||
yield
|
||||
if os.path.exists(_test_db):
|
||||
os.unlink(_test_db)
|
||||
|
||||
|
||||
def _set(monkeypatch, **kw):
|
||||
s = N._get_settings()
|
||||
for k, v in kw.items():
|
||||
monkeypatch.setattr(s, k, v, raising=False)
|
||||
|
||||
|
||||
def _mk_task(wid="ORCH-095", repo="orchestrator", title="card",
|
||||
plane_issue_id="issue-uuid-1", stage="development",
|
||||
brd_start=None, brd_end=None):
|
||||
conn = get_db()
|
||||
cur = conn.execute(
|
||||
"INSERT INTO tasks (plane_id, work_item_id, repo, branch, stage, title, "
|
||||
"plane_issue_id, brd_review_started_at, brd_review_ended_at) "
|
||||
"VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
|
||||
("p1", wid, repo, "feature/ORCH-095-x", stage, title, plane_issue_id,
|
||||
brd_start, brd_end),
|
||||
)
|
||||
tid = cur.lastrowid
|
||||
conn.commit()
|
||||
conn.close()
|
||||
return tid
|
||||
|
||||
|
||||
def _mk_run(task_id, agent, started, finished, in_tok=100, out_tok=50,
|
||||
cache_read=0, cache_creation=0, cost=0.0, model=None,
|
||||
effort=None, exit_code=0):
|
||||
conn = get_db()
|
||||
cur = conn.execute(
|
||||
"INSERT INTO agent_runs (task_id, agent, started_at, finished_at, "
|
||||
"exit_code, input_tokens, output_tokens, cache_read_tokens, "
|
||||
"cache_creation_tokens, cost_usd, model, effort) "
|
||||
"VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
|
||||
(task_id, agent, started, finished, exit_code, in_tok, out_tok,
|
||||
cache_read, cache_creation, cost, model, effort),
|
||||
)
|
||||
rid = cur.lastrowid
|
||||
conn.commit()
|
||||
conn.close()
|
||||
return rid
|
||||
|
||||
|
||||
# A stage that lasted 30s (< 60s) -> _fmt_minutes -> "<1м".
|
||||
_SUB_MIN_START = "2026-06-04 09:00:00"
|
||||
_SUB_MIN_END = "2026-06-04 09:00:30"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-01 — sub-minute duration is HTML-safe at the render boundary
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc01_sub_minute_duration_escaped_at_boundary():
|
||||
# ADR-001 D2: _fmt_minutes keeps returning the literal "<1м" (source
|
||||
# unchanged); safety comes from _esc at the boundary -> "<1м".
|
||||
assert N._fmt_minutes(30) == "<1м"
|
||||
escaped = N._esc(N._fmt_minutes(30))
|
||||
assert escaped == "<1м"
|
||||
assert "<1" not in escaped # no raw opening-tag-looking substring
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-02 — _fmt_minutes boundary inputs: never-raise + boundary-safe
|
||||
# --------------------------------------------------------------------------- #
|
||||
@pytest.mark.parametrize("value", [0, None, "abc", 59, 60, 61, 100000, -5, 30])
|
||||
def test_tc02_fmt_minutes_never_raise_and_safe(value):
|
||||
out = N._fmt_minutes(value) # must not raise
|
||||
assert isinstance(out, str)
|
||||
safe = N._esc(out)
|
||||
# After boundary escaping no raw '<' (or '>'/'&') survives in any branch.
|
||||
assert "<" not in safe
|
||||
assert ">" not in safe
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-03 — render_task_tracker for a sub-minute stage: no raw '<1м'
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc03_render_sub_minute_stage_is_safe():
|
||||
tid = _mk_task(stage="development")
|
||||
# Analysis stage lasted 30s; analysis sits before development -> ✅ line shown.
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8")
|
||||
|
||||
text = N.render_task_tracker(tid)
|
||||
assert "<1м" not in text # the bug: raw literal must be gone
|
||||
assert "<1м" in text # rendered safely instead
|
||||
# And no double escaping leaked in.
|
||||
assert "&lt;" not in text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-04 — title with '<', '>', '&' escaped, no raw tags, no double-escape
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc04_title_special_chars_escaped_no_double():
|
||||
tid = _mk_task(title="A <b>x</b> & <1", stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END)
|
||||
|
||||
text = N.render_task_tracker(tid)
|
||||
# Data special chars present only escaped...
|
||||
assert "<b>" in text
|
||||
assert "&" in text
|
||||
# ...never as raw markup from the title.
|
||||
assert "<b>" not in text
|
||||
assert "</b>" not in text
|
||||
# No double escaping anywhere.
|
||||
assert "&lt;" not in text
|
||||
assert "&amp;" not in text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-05 — status label + model + effort are escaped (defence-in-depth)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc05_status_label_escaped(monkeypatch):
|
||||
monkeypatch.setattr(N, "_card_status_label", lambda *a, **k: "<danger>")
|
||||
tid = _mk_task(stage="development")
|
||||
text = N.render_task_tracker(tid)
|
||||
assert "<danger>" in text
|
||||
assert "<danger>" not in text
|
||||
|
||||
|
||||
def test_tc05_model_escaped(monkeypatch):
|
||||
# The model name is a D-slot: even if a '<' ever reached it, it is escaped.
|
||||
import src.usage as U
|
||||
monkeypatch.setattr(U, "short_model_name", lambda m: "<m>" if m else "")
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="whatever")
|
||||
text = N.render_task_tracker(tid)
|
||||
assert "<m>" in text
|
||||
assert "<m>" not in text
|
||||
|
||||
|
||||
def test_tc05_effort_escaped():
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END,
|
||||
model="claude-opus-4-8", effort="<e>")
|
||||
text = N.render_task_tracker(tid)
|
||||
assert "<e>" in text
|
||||
assert "<e>" not in text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-06 — token / cost metrics are HTML-safe ('$' + digits)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc06_token_cost_metrics_safe():
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END,
|
||||
in_tok=1_100_000, out_tok=39_600, cost=2.38,
|
||||
model="claude-opus-4-8")
|
||||
text = N.render_task_tracker(tid)
|
||||
# The 💰 totals line renders with a '$' cost and no stray angle brackets
|
||||
# coming from the metric data.
|
||||
assert "$" in text
|
||||
assert "&" not in text or "&lt;" not in text # no double-escape
|
||||
# No raw opening-tag substring produced by the metrics.
|
||||
assert "<$" not in text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-07 — markup regression: clickable issue number stays a valid <a> tag
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc07_issue_number_stays_clickable(monkeypatch):
|
||||
_set(monkeypatch, plane_web_url="https://plane.example.org",
|
||||
plane_api_url="http://localhost:8091", plane_workspace_slug="acme")
|
||||
tid = _mk_task(plane_issue_id="abcd-issue-uuid", stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END)
|
||||
|
||||
text = N.render_task_tracker(tid)
|
||||
expected_url = (
|
||||
f"https://plane.example.org/acme/projects/{_ORCH_PROJECT_ID}"
|
||||
f"/issues/abcd-issue-uuid/"
|
||||
)
|
||||
# The anchor markup is NOT escaped (M-slot) -> still clickable & valid.
|
||||
assert f'<a href="{expected_url}">ORCH-095</a>' in text
|
||||
assert text.count("<a href=") == text.count("</a>")
|
||||
# href/label are not double-escaped.
|
||||
assert "<a href=" not in text
|
||||
assert "&lt;" not in text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-08 — markup regression: _done_link renders valid '🔗 PR #n · 📦 Внедрено'
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc08_done_link_markup_preserved(monkeypatch):
|
||||
tid = _mk_task(stage="done")
|
||||
_mk_run(tid, "deployer", _SUB_MIN_START, _SUB_MIN_END)
|
||||
|
||||
# Mock the Gitea PR lookup inside _done_link.
|
||||
resp = MagicMock()
|
||||
resp.status_code = 200
|
||||
resp.json = lambda: [{"number": 105}]
|
||||
monkeypatch.setattr(N.httpx, "get", lambda *a, **k: resp)
|
||||
|
||||
text = N.render_task_tracker(tid)
|
||||
assert "\U0001f517 PR #105" in text # 🔗 PR #105
|
||||
assert "\U0001f4e6" in text # 📦
|
||||
# The done line is an M-slot -> not escaped.
|
||||
assert "<" not in text.split("\n")[-1]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-09 — integration: edit payload for a sub-minute card is parse-safe
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc09_edit_payload_is_parse_safe(monkeypatch):
|
||||
from src.db import set_tracker_message_id
|
||||
_set(monkeypatch, tracker_mode="edit",
|
||||
telegram_bot_token="bot-token", telegram_chat_id="chat-1")
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8")
|
||||
set_tracker_message_id(tid, 18854)
|
||||
|
||||
captured = {}
|
||||
|
||||
def _fake_post(url, json=None, timeout=None, **kw):
|
||||
captured["url"] = url
|
||||
captured["json"] = json
|
||||
return SimpleNamespace(status_code=200, json=lambda: {"ok": True})
|
||||
|
||||
monkeypatch.setattr(N.httpx, "post", _fake_post)
|
||||
|
||||
N.update_task_tracker(tid)
|
||||
|
||||
assert "editMessageText" in captured["url"]
|
||||
payload_text = captured["json"]["text"]
|
||||
assert captured["json"]["parse_mode"] == "HTML"
|
||||
# The crux of ORCH-095: no raw '<1м' reaches Telegram -> no 'can't parse
|
||||
# entities' -> the card does not freeze.
|
||||
assert "<1м" not in payload_text
|
||||
assert "<1м" in payload_text
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-10 — stuck card resumes; anti-duplicate (ORCH-087) preserved
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc10_valid_render_edits_in_place_no_new_card(monkeypatch):
|
||||
from src.db import set_tracker_message_id, get_tracker_message_id
|
||||
_set(monkeypatch, tracker_mode="edit")
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8")
|
||||
set_tracker_message_id(tid, 18854)
|
||||
|
||||
# After the fix the render is valid -> edit succeeds in place (EDIT_OK).
|
||||
monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_OK)
|
||||
send_mock = MagicMock(return_value=999)
|
||||
monkeypatch.setattr(N, "send_telegram", send_mock)
|
||||
|
||||
N.update_task_tracker(tid)
|
||||
|
||||
send_mock.assert_not_called() # edited in place, no new card
|
||||
assert get_tracker_message_id(tid) == 18854 # pointer unchanged
|
||||
|
||||
|
||||
def test_tc10_transient_fail_does_not_duplicate(monkeypatch):
|
||||
# ORCH-087 invariant: a transient edit failure must NOT spawn a new card.
|
||||
from src.db import set_tracker_message_id, get_tracker_message_id
|
||||
_set(monkeypatch, tracker_mode="edit")
|
||||
tid = _mk_task(stage="development")
|
||||
_mk_run(tid, "analyst", _SUB_MIN_START, _SUB_MIN_END, model="claude-opus-4-8")
|
||||
set_tracker_message_id(tid, 18854)
|
||||
|
||||
monkeypatch.setattr(N, "edit_telegram", lambda mid, text: N.EDIT_FAILED)
|
||||
send_mock = MagicMock(return_value=999)
|
||||
monkeypatch.setattr(N, "send_telegram", send_mock)
|
||||
|
||||
N.update_task_tracker(tid)
|
||||
|
||||
send_mock.assert_not_called() # no duplicate on transient fail
|
||||
assert get_tracker_message_id(tid) == 18854
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# TC-11 — never-raise on broken inputs
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_tc11_never_raise_missing_task():
|
||||
# No such task -> minimal fallback string, no exception.
|
||||
assert N.render_task_tracker(999999) == "task-999999"
|
||||
|
||||
|
||||
def test_tc11_never_raise_none_title_and_bad_timestamps():
|
||||
tid = _mk_task(title=None, stage="development")
|
||||
# Unparseable timestamps -> _duration_seconds degrades to None, no raise.
|
||||
_mk_run(tid, "analyst", "not-a-ts", "also-bad", model="claude-opus-4-8")
|
||||
text = N.render_task_tracker(tid) # must not raise
|
||||
assert isinstance(text, str)
|
||||
assert "ORCH-095" in text # falls back to work_item_id
|
||||
|
||||
|
||||
def test_tc11_esc_never_raises():
|
||||
class _Boom:
|
||||
def __str__(self):
|
||||
raise RuntimeError("boom")
|
||||
|
||||
# _esc degrades to '' rather than propagating an exception (FR-5).
|
||||
assert N._esc(_Boom()) == ""
|
||||
assert N._esc(None) == "None"
|
||||
Reference in New Issue
Block a user