# Ревью кода: `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). Фраза пре-существующая; по решению человека дефект уходит в беклог отдельной задачей, а текст вливается в спеку как есть.