Ревью кодовой базы и исправления корректности по его итогам

Ревью кодовой базы: docs/reviews/2026-09-27-codebase-review.md.

Корректность и согласованность, пункты 5–7 ревью (docs/changes/018):
- одиночное удаление бэкапа в UI идёт через общий delete_many: пометка
  deleted_at и событие backup.deleted, как у группового удаления и API;
- единая система миграций: ручные ALTER из db._migrate перенесены в
  migrations.run (при user_version < 1, до замены ID);
- групповая смена канала выполняется фоновыми задачами set_channel;
  PUT /api/v1/batch/channel → 202 {"job_ids": [...]} (ломающее изменение
  API), меню «Канал» в UI выводит задачи в панель «Задачи».

Тесты: 22 из 22. Стенд проверен на порту 8001 (8000 занят посторонним
процессом), боевые данные не изменены. Ручная проверка UI пользователем
на момент коммита не подтверждена.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs/changes/018-correctness-consistency/plan.md

@@ -0,0 +1,80 @@
+# План: 018 — корректность и согласованность (по ревью 2026-09-27)
+
+## Context
+
+Ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Корректность и согласованность», пункты 5–7:
+
+- **п. 5** — одиночное удаление бэкапа в UI (`POST /backups/delete`, `app/ui/routes.py::backup_delete`) вызывает
+  `s3.delete_object` напрямую. Метаданные (`backups.deleted_at`) не обновляются, событие `backup.deleted` не пишется —
+  в отличие от группового удаления и API, которые идут через `backups.delete_many`.
+- **п. 6** — две системы миграций: `app/db.py::_migrate` (ручные `ALTER TABLE devices ADD COLUMN …` без версии)
+  и `app/migrations.py` (`PRAGMA user_version`). Нужна одна точка с версией схемы.
+- **п. 7** — групповая смена канала (`PUT /api/v1/batch/channel`, `POST /ui/batch/channel`) выполняется последовательно
+  внутри HTTP-запроса: на большом парке упирается в таймаут, сбой одного устройства прерывает остальные.
+
+Решения пользователя:
+- п. 7 — **фоновые задачи**: новый тип задачи `set_channel`; `PUT /api/v1/batch/channel` меняет контракт на
+  `202 {"job_ids": [...]}` (как `/batch/backup`).
+- Стенд — существующий контейнер `ros_control-ros_control-1` (compose-проект в корне репозитория). **В томе боевые данные:
+  удалять существующие устройства, группы, бэкапы, задачи и записи журнала нельзя.** Для проверок можно создавать новые
+  объекты; созданное проверкой убирается только штатными средствами, существующее не трогается.
+
+## Изменения
+
+### п. 5 — одиночное удаление через общий сервис (`app/ui/routes.py`)
+- `backup_delete`: вместо `s3.key_allowed` + `s3.delete_object` вызвать `backups.delete_many([key])`
+  (проверка ключа, удаление, `sync_rows` с событием уже внутри). Недопустимый ключ — прежний `ValueError` → 400.
+- Ответ — редирект на `/backups?deleted=N&failed=M`, как у `backups_delete_many` (страница уже показывает итог по этим параметрам).
+- API `DELETE /api/v1/backups` уже использует `delete_many` — не меняется.
+
+### п. 6 — единая система миграций (`app/db.py`, `app/migrations.py`)
+- Удалить `db._migrate` и его вызов из `init_db`.
+- В `migrations.py` добавить `_add_legacy_columns(con)`: те же три `ALTER` (`use_tls BOOLEAN NOT NULL DEFAULT 1`,
+  `group_id VARCHAR(40)`, `note TEXT`), идемпотентно по `PRAGMA table_info(devices)`.
+- Вызов в `migrations.run` **при `version < 1` до `_is_legacy`/`_to_v1`** — `_to_v1` читает `use_tls`, `group_id`, `note`
+  из старой таблицы, поэтому колонки должны появиться раньше. `SCHEMA_VERSION` остаётся 2 (схема не меняется).
+- Ветка `:memory:` (тесты) не меняется: `create_all` создаёт полную схему.
+- Боевая БД уже на `user_version = 2` — для неё изменение не выполняет никаких действий.
+
+### п. 7 — групповая смена канала через задачи (`app/services/jobs.py`, `app/services/ops.py`, `app/api/v1.py`, `app/ui/routes.py`, `app/ui/templates/dashboard.html`)
+- `ops.run_set_channel(device_id, channel) -> str`: **переиспользовать** `ops.set_channel` (установка канала + `refresh_status`);
+  возвращает сообщение «Канал <c> установлен».
+- `jobs.JOB_TYPES["set_channel"] = ops.run_set_channel`. `jobs.start_jobs(job_type, device_ids, params: dict | None = None)`:
+  `params` передаются раннеру как именованные аргументы (`await JOB_TYPES[t](device_id, **params)`) и пишутся в `data`
+  события `job.created`. Колонку в `jobs` не добавляем: незавершённые задачи после перезапуска всё равно помечаются
+  проваленными (`fail_stale_jobs`), повторно параметры не нужны.
+- Проверка канала — до создания задач: `channel not in CHANNELS` → `ValueError` (400); в API и так `Literal[CHANNELS]`.
+- API: `PUT /api/v1/batch/channel` → `status_code=202`, ответ `{"job_ids": jobs.start_jobs("set_channel", body.resolve(), {"channel": body.channel})}`.
+  `PUT /api/v1/devices/{id}/update/channel` (одно устройство) — **без изменений**, синхронный.
+- UI: `POST /ui/batch/channel` — создаёт задачи и возвращает `_jobs.html`, как остальные групповые действия;
+  в `dashboard.html` у кнопок меню «Канал» `hx-target="#jobs"`, `hx-include="[name=device_ids]:checked"` (как у «Обновление»).
+  Смена канала у одного устройства из меню строки (`/ui/devices/{id}/channel`) — без изменений.
+- Таблица устройств обновится автоопросом (`/ui/devices?poll=1`) после завершения задач — `refresh_status` уже записывает статус.
+- Отображение типа задачи `set_channel` в панели «Задачи» и журнале — проверить подписи типов (если есть словарь подписей — добавить «Смена канала»).
+
+## Тесты (минимально)
+- В существующий тест группы/устройств или новый короткий: `PUT /api/v1/batch/channel` → 202 и `job_ids` по числу устройств;
+  раннер подменён (`monkeypatch` на `ops.set_channel`), задача завершается `done`, в `job.created` есть `{"channel": …}`.
+- `test_migration_replaces_numeric_ids`: убедиться, что тест по-прежнему покрывает старую схему без колонок `use_tls/group_id/note`
+  (если его фикстура создаёт их сама — добавить вариант без них).
+- `test_bulk_delete_backups`: одна проверка `POST /backups/delete` → редирект с `deleted=1`, вызов идёт через `delete_many`.
+
+## Документация
+- README: таблица API (`PUT /api/v1/batch/channel` → 202 `{"job_ids"}`), типы задач, строка 018 в «История изменений»;
+  число тестов в разделе «Разработка».
+- `summary.md` — оркестратор после проверки.
+
+## Исполнение
+- Код, тесты, пересборка стенда — исполнитель (Sonnet). Тесты не запускает, не коммитит, `summary.md` не создаёт.
+- Оркестратор: ревью диффа, полный прогон тестов, проверки на стенде на **новых** тестовых объектах без удаления существующих данных.
+- Пользователь: ручная проверка UI.
+
+## Проверка
+- `./venv/bin/python -m pytest -q` — все зелёные.
+- Стенд: `docker compose up -d --build --force-recreate`, контейнер `ros_control-ros_control-1` запущен,
+  `docker exec … grep -c set_channel /srv/app/services/jobs.py` ≥ 1, `PRAGMA user_version` = 2, число строк в таблицах до и после совпадает.
+- Сценарии на стенде (новое тестовое устройство с недоступным адресом, например `192.0.2.1` из TEST-NET):
+  `PUT /api/v1/batch/channel` → 202, задача `set_channel` завершается `failed` с ошибкой соединения, остальные задачи не затронуты;
+  неверный канал → 422; тестовое устройство затем удаляется через API (только оно).
+- Миграция старой схемы — тестом на временной БД (боевую не трогаем).
+- Ручная проверка UI — пользователь: меню «Канал» при выбранных устройствах создаёт задачи в панели «Задачи»; удаление одного файла на «Бэкапах» показывает итог.

docs/changes/018-correctness-consistency/summary.md

@@ -0,0 +1,29 @@
+# Итоги: 018 — корректность и согласованность (по ревью 2026-09-27)
+
+Источник — ревью `docs/reviews/2026-09-27-codebase-review.md`, раздел «Корректность и согласованность» (пункты 5–7).
+
+## Сделано
+- **п. 5 — одиночное удаление бэкапа** (`app/ui/routes.py::backup_delete`): идёт через `backups.delete_many([key])`, как групповое и API.
+  Проверка ключа, удаление, `sync_rows` (пометка `deleted_at`, событие `backup.deleted`) теперь общие; ответ — редирект `/backups?deleted=N&failed=M`.
+- **п. 6 — единая система миграций**: `db._migrate` удалён; колонки старой схемы (`use_tls`, `group_id`, `note`) добавляет
+  `migrations._add_legacy_columns` при `user_version < 1` **до** `_to_v1` (та читает эти колонки). `SCHEMA_VERSION` = 2, схема не менялась.
+- **п. 7 — групповая смена канала фоновыми задачами**: тип задачи `set_channel` (`ops.run_set_channel` поверх `ops.set_channel`);
+  `jobs.start_jobs(job_type, device_ids, params)` передаёт `params` раннеру и в `data` события `job.created`.
+  `PUT /api/v1/batch/channel` → **202 `{"job_ids": [...]}`** (было: синхронный список устройств). UI: меню «Канал» создаёт задачи и обновляет панель «Задачи»;
+  подпись «Смена канала» в таблице задач. Смена канала одного устройства (API и меню строки) не менялась.
+- README: таблица API, число тестов, строка 018 в истории изменений, каталог `docs/reviews`.
+
+## Проверено
+- `pytest`: 22 из 22 (новые: `test_batch_channel_runs_as_jobs`, `test_migration_adds_legacy_columns_before_id_migration`; `test_bulk_delete_backups` дополнен одиночным удалением).
+- Стенд `ros_control-ros_control-1` пересобран, отдаёт новый код (`grep set_channel` в контейнере), `/login` → 200, старт без ошибок.
+- Сценарий на стенде с временным устройством `192.0.2.1`: `PUT /batch/channel` → 202; задача `set_channel` → `failed` (`ConnectTimeout`, ожидаемо);
+  в `job.created` записан `{"channel": "stable"}`; неверный канал → 422; временное устройство удалено (204).
+- Боевые данные: группы 4/4, устройства 12/12, бэкапы 41/41 — совпадают по ID до и после; добавились только 1 задача и 7 событий проверки; `user_version` = 2.
+
+## Оговорки
+- **Ломающее изменение API**: `PUT /api/v1/batch/channel` возвращает 202 и ID задач вместо списка устройств — клиентам нужно опрашивать `/api/v1/jobs/{id}`.
+- Порт 8000 на хосте занят посторонним процессом (`telemetry_web`), поэтому стенд временно запущен на 8001 через override-файл вне репозитория;
+  `docker-compose.yml` не менялся. После освобождения порта — `docker compose up -d --force-recreate`.
+- Одиночное удаление на стенде не выполнялось (удалило бы реальный файл из бакета) — покрыто тестом.
+- Ручная проверка UI пользователем на момент коммита не подтверждена.
+- Записи проверки (тестовое устройство, задача, события) остались в журнале как обычные события.

docs/reviews/2026-09-27-codebase-review.md

@@ -0,0 +1,78 @@
+# Ревью кодовой базы ros_control — 2026-09-27
+
+Состояние на коммит `26dd1f8`. Тесты: 20/20 проходят.
+
+## Назначение
+
+Централизованное управление парком MikroTik RouterOS: опрос статуса, бэкапы в S3 (Yandex Object Storage),
+обновление ROS и прошивки, группы устройств, журнал событий. Объём — около 2,5 тыс. строк (Python и шаблоны).
+
+## Архитектура
+
+| Слой | Модули | Назначение |
+|---|---|---|
+| Вход | `app/main.py` | FastAPI; при старте запускает опрос устройств, сверку бэкапов с бакетом и ротацию журнала; ошибки переводятся в HTTP-коды 404, 400 и 502 |
+| API | `app/api/v1.py` | REST под Bearer-токеном |
+| UI | `app/ui/routes.py`, шаблоны | Jinja2 + HTMX, сессия в cookie, один администратор |
+| Сервисы | `app/services/*` | `devices`, `groups`, `jobs` (фоновые задачи), `ops` (связка устройство + S3 + БД), `poller`, `backups`, `events`, `settings` |
+| Интеграции | `app/ros/client.py`, `app/ros/operations.py`, `app/s3.py` | REST-клиент RouterOS на httpx; boto3 вызывается через `to_thread` |
+| Данные | `app/models.py`, `app/db.py`, `app/migrations.py`, `app/ids.py` | SQLite; ID вида `<префикс>_<uuid7>`; версия схемы в `PRAGMA user_version` |
+
+API и UI используют один и тот же сервисный слой, поэтому логика не дублируется. Операции с устройствами
+(`app/ros/operations.py`) не зависят от БД и S3, поэтому их легко тестировать. Паролей устройств нет в ответах
+API, в БД они зашифрованы (Fernet).
+
+## Сильные стороны
+
+- **Бэкап:** файлы скачиваются с устройства по REST блоками в base64 со сверкой размера, затем уходят в S3.
+  В `finally` файлы удаляются с устройства и при успехе, и при сбое.
+- **Опрос:** быстрый режим — один запрос `system/resource`. Полная проверка обновлений идёт по отдельному
+  интервалу. Короткий таймаут соединения позволяет быстро замечать недоступные устройства.
+- **Журнал событий:** автор действия передаётся через ContextVar. Запись события идёт в одной транзакции
+  с изменением. Есть ротация и постраничный вывод по ID.
+- **Миграция на новые ID:** перед ней делается копия файла БД, всё выполняется одной транзакцией
+  со сверкой числа строк.
+
+## Замечания
+
+### Безопасность
+
+| № | Замечание | Где | Приоритет |
+|---|---|---|---|
+| 1 | Небезопасные значения по умолчанию не блокируются: `API_TOKEN`, `ADMIN_PASSWORD`, `SESSION_SECRET` = `change-me`. Если `.env` не заполнен, API открыт с известным токеном. Стоит отказываться запускаться при таких значениях | `app/config.py` | Высокий |
+| 2 | Вход в UI без защиты от перебора паролей. Блокировка после неверных попыток (`security.register_failure`) есть только при очистке журнала | `app/ui/routes.py`, `login` | Высокий |
+| 3 | Сессионная cookie без флага Secure (`https_only=False`). Нужен TLS-терминатор перед сервисом и флаг, включаемый через настройки | `app/main.py` | Средний |
+| 4 | Открытый редирект в `/ui/move`: значение `next=/\evil.com` проходит проверку, а браузер воспринимает его как адрес другого сайта. Риск низкий: запрос только POST, cookie с `SameSite=strict` | `app/ui/routes.py`, `move` | Низкий |
+
+### Корректность и согласованность
+
+| № | Замечание | Где | Приоритет |
+|---|---|---|---|
+| 5 | Одиночное удаление бэкапа в UI вызывает `s3.delete_object` напрямую, в обход `backups.delete_many`. В итоге не обновляются метаданные (`deleted_at`) и не пишется событие, в отличие от API и группового удаления | `app/ui/routes.py`, `backup_delete` | Средний |
+| 6 | Две системы миграций: `db._migrate` (ручные `ALTER`) и `migrations.py` (`user_version`). Лучше свести их в одну | `app/db.py`, `app/migrations.py` | Низкий |
+| 7 | Смена канала обновлений у группы устройств идёт последовательно и синхронно внутри HTTP-запроса. На большом парке запрос упрётся в таймаут, а ошибка на одном устройстве прерывает остальные | `/api/v1/batch/channel`, `/ui/batch/channel` | Средний |
+
+### Производительность и масштабирование
+
+| № | Замечание | Где | Приоритет |
+|---|---|---|---|
+| 8 | Каждое открытие страницы бэкапов перечитывает весь бакет и запускает `sync_rows` по всей таблице `backups`. Нагрузка растёт линейно с числом копий и оплачивается запросами к S3 | `app/services/backups.py`, `search` | Средний |
+| 9 | Синхронные запросы к БД внутри async-обработчиков. На SQLite и небольших объёмах это терпимо, но под нагрузкой блокирует event loop | сервисный слой | Низкий |
+| 10 | Состояние хранится в памяти процесса: семафор задач, `_sync_lock`, счётчики неудачных попыток пароля. Приложение рассчитано на один процесс, `--workers > 1` сломает эти гарантии. Стоит явно указать это в README | `jobs`, `backups`, `security` | Низкий |
+
+### Эксплуатация
+
+| № | Замечание | Где | Приоритет |
+|---|---|---|---|
+| 11 | Нет healthcheck, логирование не настроено (используется стандартный `logging`) | `Dockerfile`, `docker-compose.yml` | Низкий |
+| 12 | Тесты собраны в одном файле (570 строк). Это соответствует требованию минимума тестов, но при росте проекта файл станет трудно поддерживать | `tests/test_app.py` | Низкий |
+
+## Рекомендуемый порядок исправлений
+
+1. Пункты 1 и 2: проверка небезопасных значений при старте и защита входа от перебора. Это небольшие изменения,
+   которые сильнее всего снижают риск.
+2. Пункт 5: одиночное удаление через `delete_many`.
+3. Пункт 7: групповая смена канала через фоновые задачи (`jobs`).
+4. Пункт 8: кэшировать список объектов бакета или вызывать `sync_rows` только после изменений.
+
+Каждое исправление оформляется отдельным изменением в `docs/changes/` (план → доработка → итоги → README).