From b38cc160410a25424f995f67e6b0c9c8f2ed5fa7 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 00:07:17 +0300 Subject: [PATCH] fix(notifications): escape all card data fields at the render boundary (ORCH-095) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 --- CHANGELOG.md | 7 + src/notifications.py | 55 +++-- tests/test_tracker_html_escape.py | 358 ++++++++++++++++++++++++++++++ 3 files changed, 406 insertions(+), 14 deletions(-) create mode 100644 tests/test_tracker_html_escape.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 624f669..4ae9791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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-безопасность токенов/стоимости, регресс кликабельного `` номера и `_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`) сохранены. diff --git a/src/notifications.py b/src/notifications.py index dbc6b31..bc82174 100644 --- a/src/notifications.py +++ b/src/notifications.py @@ -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}" ) diff --git a/tests/test_tracker_html_escape.py b/tests/test_tracker_html_escape.py new file mode 100644 index 0000000..1c9ebdb --- /dev/null +++ b/tests/test_tracker_html_escape.py @@ -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 -> ), 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 x & <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 "" not in text + assert "" 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: "") + tid = _mk_task(stage="development") + text = N.render_task_tracker(tid) + assert "<danger>" in text + assert "" 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: "" 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 "" 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="") + text = N.render_task_tracker(tid) + assert "<e>" in text + assert "" 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 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'ORCH-095' in text + assert text.count("") + # 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"