jellybit/openspec/changes/archive/2026-08-10-bulk-delete-page/review/report.md

# Ревью кода: `bulk-delete-page` — итог триажа (стадия `large`)

## Сводка

- **Размер / сложность / метка:** крупное / незнакомое / **large**. Метка
  поднята с `medium` повторной разметкой после правок дельта-спек: изменение
  трогает `store` + `worker` + `httpapi` + `tgbot` + шаблоны и заводит новую
  точку входа поверх необратимой операции.
- **Режим прогона:** по графу.
- **Гейт:** зелёный после всех правок, diff-coverage 89 %.
- **Находок на входе:** 10 новых (`architecture` A1–A3, `adversary` S1–S4,
  `ops` O1–O3) + 6 перенесённых из отчёта `medium`-стадии. **На выходе:** 6.

Прогон шёл в две стадии. Сперва `medium` (autotests, specs, code, basics,
triage) — его находки отработаны:

- отказ чтения хранилища отличён от отсутствия записи (строка «состояние
  прочитать не удалось», лог, сценарий спеки);
- строки отчёта читаются неотменяемым контекстом;
- отсутствие записи явно названо в спеке не-системным отказом;
- три sentinel'а разбора пачки переехали в `classifyErr`, таблица
  `docs/conventions/errors.md` дополнена;
- добавлен оракул на ссылку `/delete` в шапке;
- по решению человека введён потолок времени на проход (`bulkBudget`);
- по решению человека спека перестала обещать, что повтор отправки ничего не
  делает.

### Тема без отчёта на этой стадии — находка о прогоне

**Три прохода `medium`-стадии (`autotests`, `specs`, `code`) после правок не
перезапускались, а правки затронули их предмет.**

- `specs` (тема **requirements**) отчитывался по **прежнему** тексту дельта-спек.
  После него дельты изменены трижды. Соответствие кода изменённому тексту не
  проверял никто.
- `code` (тема **conventions**) отчитывался до переезда sentinel'ов в
  `classifyErr` и до правки `docs/conventions/errors.md`.
- `autotests` — гейт перепрогнан целиком и зелёный; вопрос темы «есть ли тест,
  который упал бы без этой правки» на новых строках заново не задавался.

Это не «проходы не нашли», а «проходы не смотрели».

### План разметки с исходом по каждой теме

| тема | дом | глубина | закрывает | исход |
|---|---|---|---|---|
| requirements | `openspec/specs/{web-ui,state-reconciliation}` + дельты | разбор | `specs` | отчёта на этой стадии нет; закрыта на `medium` (3 находки), текст дельт после этого изменён |
| autotests | `CLAUDE.md` → «Гейт» | — | `autotests` | отчёта на этой стадии нет; гейт перепрогнан, зелёный |
| conventions | `docs/conventions/` | разбор | `code` | отчёта на этой стадии нет; закрыта на `medium` (3 находки) |
| architecture | `docs/architecture.md` + источник `docs/passport.md` | доказательство | `architecture` | **закрыта**, 3 находки (A1–A3) |
| security | `docs/security.md` | доказательство | `adversary` | **закрыта**, 4 находки (S1–S4) |
| operations | `docs/architecture.md` «Эксплуатация» + источник `docs/database.md` | доказательство | `ops` | **закрыта**, 3 находки (O1–O3) |
| тема проекта | своих тем в `docs/` нет | — | — | дома у темы нет |

---

## Блокирует мердж

### 1. Строка `done` уезжает в пачку без отметки последней копии — единственная копия медиафайла стирается по подтверждению, которое об этом промолчало

- Файл: `internal/httpapi/bulkdelete.go:113,220`; причина —
  `internal/worker/reconcile.go:28-41,56-67`, `internal/layout/layout.go:490-510`
- Severity: `major`; Confidence: high; проход `adversary` (S1)
- **Оракул** — два падающих теста (`go test -overlay`):
  `TestAdversaryDoneRowIsLastCopyWithoutWarning`,
  `TestAdversaryDoneRowConfirmedWithoutLastCopyWarning` («ни страница выбора, ни
  подтверждение не сказали о последней копии… отчёт "Удалено — 1"»).
  Механизм сверен триажем: `LastCopy: d.State == store.StateOrphaned`;
  `sourceSeen` берётся исключительно из присутствия хеша в ответе
  `torrents/info`, байты на диске сверка не щупает, поэтому
  `deriveState(true, true) == done` держится сколько угодно долго;
  `layout.Remove` не зовёт `isLastCopy` вовсе.
- Последствие: задача в `done`, у которой байты источника исчезли с диска, а
  раздача осталась в списке qBittorrent (ручная уборка каталога, отвалившийся
  том, торрент в `missingFiles`), предлагается к удалению без единственного
  предохранителя, который дельта-спека для этого и заводит. `Undo` на том же
  входе отказал бы целиком. Позиция №1 шкалы `docs/security.md`, инвариант
  «последняя копия не снимается» — **необратимо**. Спека и решение D2 исходят из
  тождества «`orphaned` ⟺ `nlink<=1`»; оно неверно в одну сторону.
- **Действие: развилка.** → **решение человека (2026-08-10): оставить как есть,
  назвать границу в спеке.** Отметка выводится из состояния, случай «`done` с
  пропавшими байтами источника» ею не покрыт, и это записано прямо в требовании
  о поимённом подтверждении. Довод: поштучный путь такой отметки не несёт
  вовсе — пачка не ухудшила положение, а улучшила его не до конца.

### 2. Подтверждённое удаление старой закрытой задачи сносит источник у другой, живой загрузки с тем же инфохэшем

- Файл: `internal/worker/review.go:665-669`; причина —
  `internal/store/download.go:325-371`
- Severity: `major`; Confidence: high; проход `adversary` (S2)
- **Оракул** — два падающих теста:
  `TestAdversaryDeletablePageOffersHashOwnedByActiveDownload`,
  `TestAdversaryDeleteWipesSourceOfAnotherActiveDownload` («`Delete(A)` снёс
  раздачу с файлами, хотя тем же инфохэшем владеет активная загрузка B в
  состоянии `downloading`»). `CreateDownloadIfNoActive` ищет владельца только
  среди активных, поэтому терминальная A и активная B по одному хешу — легальное
  состояние; `ListDeletableDownloads` фильтрует только по состоянию;
  `qbt.Delete` вызывается без проверки активного владения, хотя предикат в
  домене есть — `store.FindActiveByInfohash`.
- Последствие: человек подтверждает удаление одной сущности, необратимое
  действие применяется к другой — класс из журнала `docs/review.md`
  (2026-08-06). **Дефект предсуществует в поштучном `Delete`**; изменение его не
  вносит, но снимает естественную остановку «человек открыл карточку и
  посмотрел».
- **Действие: развилка.** → **решение человека (2026-08-10): в беклог отдельной
  задачей.** Дефект целиком в поштучном `Delete`, изменение его не вносит;
  чинить его здесь значит превратить задачу про новую страницу в переборку
  удаления.

### 3. Пачка удалений морозит весь сервис до ~145 секунд, и предохранитель от этого стоит в транспорте, компенсируя свойство ядра

- Файл: `internal/httpapi/bulkdelete.go:30-39,156-167`; причина —
  `internal/worker/review.go:619-621`
- Severity: `major`; Confidence: high; проходы `ops` (O1), `architecture` (A1) и
  `basics` прошлой стадии — три раза одна причина
- **Оракул** — замер `ops` на настоящем `*worker.Worker`: при задержке
  `qbt.Delete` 300 мс конкурентная `Cancel(unknown-id)` через тот же worker
  заблокирована на `280.450631ms`. Плюс падающий тест прошлой стадии
  `TestTriageDeleteHoldsWorkerLockAcrossQbt`. Сверено триажем:
  `internal/worker/worker.go:9-10` и `docs/architecture.md` называют `w.mu`
  **per-download**, фактически это один мьютекс на весь воркер; `worker.go:387-390`
  — «Медленные вызовы идут ВНЕ `w.mu`»; таймаут клиента qBittorrent — зашитые
  30 с, поля в конфиге нет. Отсюда арифметика: `bulkBudget` проверяется между
  единицами, значит реальный потолок — 120 + до 30 секунд.
- Последствие: всё это время стоят `Poll` (→ сверка), `processCatched`,
  уведомления и команды остальных транспортов. Плюс архитектурная цена: в
  `httpapi` живут три доменных правила и понятие «невыполненный остаток»;
  поштучный путь той же защиты не имеет; групповой `Dismiss` либо скопирует
  ~60 строк, либо тихо обойдётся без них.
- **Действие: развилка.** → **решение человека (2026-08-10): в беклог отдельной
  задачей.** Свойство целиком пре-существующее (поштучное удаление морозит
  сервис до 30 с той же причиной); бюджет времени остаётся как ограничение
  сверху, сужение замка — отдельная работа со своей спекой.

---

## Стоит исправить сейчас

### 4. При недоступном qBittorrent задача остаётся `done`, хотя раскладки в Jellyfin уже нет, и сама не выправится до подъёма соседа

- Файл: `internal/worker/review.go:634-670`; причина —
  `internal/worker/worker.go:635-639`
- Severity: `major`; Confidence: high; проход `ops` (O2)
- **Оракул** — замер: `down (unreachable) Delete: elapsed=524.545µs
  err=connection refused`; `bulkFailThreshold=3` останавливает пачку за
  миллисекунды (6 ERROR-строк, шторма нет). Сверено триажем: `Poll` начинается с
  `w.qbt.Torrents(ctx, "")` и при ошибке возвращается на первой же строке,
  поэтому `reconcileDesync` — а с ним коррекция `done → target_missing`, на
  которую рассчитывают D9 и спека, — не вызывается ни разу, пока сосед лежит.
- Последствие: ссылки сняты и `file_link` удалены, раздача не снесена, место не
  освобождено, состояние говорит `done`. Расхождение живёт весь простой соседа.
- **Действие: развилка.** → **решение человека (2026-08-10): в беклог отдельной
  задачей.** Порядок шагов и ранний выход `Poll` — пре-существующие, поштучный
  путь ведёт себя ровно так же.

### 5. Bidi-символы в имени раздачи уезжают в экран подтверждения дословно

- Файл: `internal/worker/discover.go:55`, `internal/httpapi/httpapi.go:701-709`
- Severity: `minor`; Confidence: high; проход `adversary` (S3)
- **Оракул** — падающий тест `TestAdversaryConfirmRendersBidiTitleVerbatim`:
  U+202E уехал дословно и на `/delete`, и на `/ui/delete/confirm`.
- Последствие: поимённое чтение заголовков — единственный предохранитель
  необратимой пачки; подменённый порядок символов делает его ненадёжным.
- **Действие: инлайн.** → **исправлено**: `displaySafe` снимает `unicode.Cf` и
  управляющие на показе (не на записи), оракул добавлен.

### 6. `docs/security.md` объявляет несуществующий пробел

- Файл: `docs/security.md:106-107`
- Severity: `minor`; Confidence: high; проход `adversary` (наблюдение)
- **Оракул** — сверено по исходникам: `internal/llm/openai.go:23`
  `maxResponseBody = 8 << 20`, `internal/metadata/http.go:41` — 4 MiB. Документ
  утверждал «Лимита на размер ответа LLM нет — известный пробел».
- **Действие: инлайн.** → **исправлено**: оба лимита названы числами в
  `docs/security.md`, вопрос темы в `docs/review.md` переформулирован.

---

## Гипотезы без доказательства

- **S4 (`adversary`, `major` → гипотеза): исходный путь раскладки не проверяется
  на принадлежность песочнице.** Тест `TestAdversarySourceEscapesDownloadsSandbox`
  разложил `…/config/config.toml` как `…/movies/Дюна (2021)/Дюна (2021).toml`
  через `..` в имени файла раздачи. **Вне диффа целиком** и с недостающим
  звеном, названным самим проходом: согласится ли qBittorrent отдать в
  `/torrents/files` имя с `..` (libtorrent такие пути санитизирует). Уезжает в
  урожай задачей.
- **O3 (`ops`, `minor` → гипотеза): страница `/delete` раздувается без
  ретеншена.** Замер: 5000 строк → 2 241 604 байта, 29.7 мс. На обозримом
  горизонте предела не надо; отсутствие пагинации заказано спекой дословно;
  смежная причина стоит задачей `db-retention-cleanup`.
- **A2 (`architecture`, `minor` → снято): «третий способ выбрать загрузки по
  набору состояний».** Перечень берётся из единой точки; `ListDownloadsByState`
  делает `SELECT *` без `LEFT JOIN recognition` и с другим `ORDER BY` —
  переиспользованию не подлежит без правки обоих вызывающих.
- **A3 (`architecture`, `minor` → promote): строка загрузки размножена по трём
  шаблонам.** Норма говорит о паре «страница ↔ htmx-фрагмент одного
  обработчика»; здесь три разные страницы с разной семантикой строки и без htmx.
  Второй независимый провенанс повышает приоритет правила, `confidence` — нет.

---

## Promote candidates

- Партиал строки — один на все экраны многостраничной формы (`code`,
  `architecture`, независимо).
- Таймаут клиента qBittorrent — из конфига, а не дефолт транспорта: в
  `[qbittorrent]` поля нет, в `[llm]`, `[metadata.*]`, `[jellyfin]` — есть
  (триаж).
- Комментарий пакета `worker` и `docs/architecture.md` называют `w.mu`
  per-download блокировкой, а это единый мьютекс на весь воркер (`ops`).
- `store.State.CanDelete()` — в `docs/architecture.md` → «Единые точки проекта»
  рядом с `IsObservable()` (`architecture`).
- Правило именования маршрутов веб-UI: глагол-первым `/delete` против семьи
  `/downloads/{id}/…` (`architecture`).
- Дубль построения `IN (?,…)` в `store` — хелпер при третьем появлении
  (`architecture`).
- Отвязка необратимой операции от контекста запроса — правило, а не приём:
  групповой путь делает `context.WithoutCancel`, поштучный нет (`code`).
- Отказы `store` не проверяются ни одним тестом — как класс (`autotests`).

---

## Границы покрытия

**Метка `large`, режим по графу.** Запущено на этой стадии: `architecture`,
`adversary`, `ops` — все на глубине «доказательство», все вернули отчёт.

**Что не запускалось и почему:** `autotests`, `specs`, `code` — сознательно не
перезапускались после правок `medium`-стадии; цена названа первой строкой
сводки. `basics` — не запускался по плану: все шесть тем ядра разобраны
именными проходами, поэтому возражений о метке от корректора прийти не могло.

**Чего запущенные проходы не могли проверить в принципе:** `architecture` не
судит корректность кода и не строит путей отказа; `adversary` показывает
достижимость, но не частоту; `ops` меряет на синтетическом входе своего
прогона, а не на рабочем потоке.

**Потолки проходов.** Ни один из трёх проходов не сообщил своего потолка.
Молчание неотличимо от «срезать было нечего» — это находка о прогоне. Триажу
пришли сжатые пересказы выводов, а не дословные блоки `Coverage of this pass`.
Потолком триажа не срезано ничего: на входе 10 новых находок, в первые две
секции ушло 6, остальные названы поимённо.

**Что остаётся целиком на человеке:** история инцидентов на umbar; поведение
SQLite под реальным объёмом и профилем нагрузки; завязка внешних потребителей на
новые маршруты; суждение «этой функциональности не должно существовать»;
качество распознавания.

**Перестали проверять сознательно:** идиоматичность Go (проход `idiom`
упразднён 2026-08-04); на метках `small`/`medium` не проверяется ничто,
требующее запуска — здесь метка `large`, и именно поэтому появились находки 1–4;
**на этом прогоне добавилось третье, разовое** — темы `requirements`,
`conventions`, `autotests` не перепроверялись после правок.

**Каких документов не хватило:** `docs/security.md` содержал устаревший факт
(находка 6, исправлено); `docs/architecture.md` и комментарий пакета `worker`
описывают `w.mu` неверно — проходы рассуждали против описания, а не против кода,
и это пришлось сверять триажу. `docs/review.md` → «Типовые ложноположительные» и
журнал дефектов есть и применены.

**Чего в конвейере нет вовсе:** решения проекта (`docs/adr/`) не сверялись —
процессный документ, расхождение ловит сверка документации; записанные
наблюдения (`docs/research/`) не использовались — всякое число снято на этом
прогоне; поимённая сверка с руководствами по стилю языка не задавалась ни одним
проходом; альтернативной реализации, с которой можно сдиффить решения, у
конвейера нет.

**Формулировка «критичных проблем не обнаружено» к этому отчёту не применима:**
проверено ровно то, что перечислено выше, и не проверено ровно то, что
перечислено выше.

---

## Досверка `specs` после правок (закрывает названную выше дыру прогона)

Проход `requirements` перезапущен на изменённом тексте дельт и изменённом коде —
дыра «соответствие кода изменённому тексту спеки не проверял никто» закрыта.
Покрытие: все 6 требований обеих дельт по под-пунктам, `openspec validate
--strict` — valid, тесты затронутых пакетов зелёные.

**Д1 (major, high). Чистка заголовка задела все заголовки веб-UI и снимала
лишнее.** `internal/httpapi/httpapi.go`. Оракул — прогон копии функции:
`"👨‍👩‍👧 family"` → `"👨👩👧 family"`, `"shah\u200Cname"` → `"shahname"`,
`"Duna\nchast 2"` → `"Dunachast 2"`. `unicode.Cf` — это не только
bidi-override, но и ZWJ/ZWNJ; поиск по списку идёт по сохранённому имени, и
скопированное с экрана название своей же записи не нашло бы. Правило нигде не
записано и применено непоследовательно (третья ветка заголовка чистку не
проходила). → **исправлено**: снимается только `unicode.Bidi_Control`,
управляющие заменяются пробелом, чистка применяется ко всем трём веткам, и
заведено MODIFIED-требование «Заголовок загрузки из имени раздачи» со сценарием.
Тест расширен проверкой, что составные эмодзи остаются целыми.

**Д2 (minor, high). Потерянный ответ пачки не восстанавливался по журналу.**
Требование «Исход каждой единицы SHALL попадать в журнал» держалось на воркере,
который отказы по конфликту и отсутствию записи пишет на `DEBUG`, а не начатые
единицы не пишет вовсе; итоговая строка несла только счётчики. → **исправлено**:
транспорт пишет строку `bulk delete item` на `INFO` по каждой единице с исходом
`deleted|failed|skipped` и причиной отказа.

**Д3 (minor, high). Отказ разбора тела формы диагностировался как
«некорректный идентификатор», исходная ошибка не доезжала ни до человека, ни до
лога.** → **исправлено**: отдельный sentinel с своим текстом, исходная ошибка
уходит в приватный канал.

**Поведение вне спеки, снятое заодно:** признак подтверждения и пачка
принимались и из строки запроса (`r.Form`), тогда как требование говорит «в
форме» — читаем только тело (`r.PostForm`).

**Осталось открытым и названо:** текст MODIFIED-требования
`state-reconciliation` обещает, что повторное удаление опирается на «приведённый
сверкой к реальности `target_missing` (кратковременное рассогласование до тика
сверки ожидаемо)». При лежащем qBittorrent это неверно — сверка не доходит до
коррекции (находка 4). Фраза пре-существующая; по решению человека дефект уходит
в беклог отдельной задачей, а текст вливается в спеку как есть.