fix(notifications): escape all card data fields at the render boundary (ORCH-095)
render_task_tracker sends/edits the live card with parse_mode=HTML. _fmt_minutes returns the literal "<1м" for a sub-minute stage; interpolated raw into HTML text Telegram parsed "<1м" as an opening tag -> editMessageText 400 can't parse entities -> edit_telegram EDIT_FAILED -> update_task_tracker early return (anti-duplicate ORCH-087) -> the card froze (incident ORCH-093, message_id 18854). Close the whole "unescaped data in HTML text" class per ADR-001: a module-local _esc(x)=html.escape(str(x)) (never-raise) wraps every DATA slot (durations, status label, model, effort, token/cost metrics) exactly once at the render boundary in render_task_tracker/_stage_line. Source functions stay HTML-agnostic (_fmt_minutes still returns "<1м"; escape on the boundary renders it visually identical as <1м, so the visible format is unchanged). Intentional MARKUP slots (num_html / link_for / _done_link / already-escaped esc_title) are NOT escaped, so the issue number stays a clickable <a> tag and nothing is double-escaped. A previously-frozen card auto-recovers on the next stage transition (a new safe render edits in place, 200) — no new code, no touch to edit_telegram / update_task_tracker / the orphan ledger, so the ORCH-087 anti-duplicate invariant is preserved (a transient edit failure still does not spawn a new card). STAGE_TRANSITIONS / QG_CHECKS / check_* / notification transport / DB schema are untouched. New tests/test_tracker_html_escape.py (TC-01..TC-11); full suite green. Refs: ORCH-095 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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`) сохранены.
|
||||
|
||||
@@ -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