diff --git a/README.md b/README.md index 3f10851..2947b01 100644 --- a/README.md +++ b/README.md @@ -213,4 +213,5 @@ venv/bin/python -m pytest -q tests/test_backups.py # один модуль ## Отчёты ревью - [Ревью кодовой базы 2026-09-27](docs/reviews/2026-09-27-codebase-review.md) (→ 018–021) - [Повторное ревью 2026-09-28 12:43](docs/reviews/2026-09-28-1243-codebase-review.md) (→ 022, 024) -- [Третье ревью 2026-09-28 17:35](docs/reviews/2026-09-28-1735-codebase-review.md) (открыты п. 11, 12, 14–19) +- [Третье ревью 2026-09-28 17:35](docs/reviews/2026-09-28-1735-codebase-review.md) (→ 025, 026) +- [Четвёртое ревью 2026-09-28 21:32](docs/reviews/2026-09-28-2132-codebase-review.md) (открыты п. 11, 13, 15, 19; п. 18 отложен) diff --git a/docs/reviews/2026-09-28-2132-codebase-review.md b/docs/reviews/2026-09-28-2132-codebase-review.md new file mode 100644 index 0000000..f2e7557 --- /dev/null +++ b/docs/reviews/2026-09-28-2132-codebase-review.md @@ -0,0 +1,69 @@ +# Ревью кодовой базы ros_control — 2026-09-28 21:32 MSK + +Четвёртое ревью. Состояние на коммит `e1f197c`. Тесты: 36/36 проходят. +Предыдущие ревью: [`2026-09-27`](2026-09-27-codebase-review.md), [`2026-09-28 12:43`](2026-09-28-1243-codebase-review.md), +[`2026-09-28 17:35`](2026-09-28-1735-codebase-review.md) (коммит `6ff7f1b`). + +## Итог + +Из 19 замечаний закрыто 15, частично — 1 (п. 13), открыто 3 (п. 11, 15, 19) и 1 отложено (п. 18). +С предыдущего ревью закрыты п. 12, 14, 16 и подтверждён на практике п. 17. + +| Изменение | Коммит | Пункты | +|---|---|---| +| `018-correctness-consistency` | `123b5ab` | 5, 6, 7 | +| `019-performance-scaling` | `ba8ac7b` | 8, 9 (частично), 10 | +| `020-custom-select-menus` | `6b5c521` | — (доработка UI по запросу) | +| `021-security-hardening` | `c407972` | 1, 2, 3, 4 | +| `022-async-db-remainder` | `0bda003` | 9 (остаток) | +| `023-ros-downgrade` | `ae826fc` | — (отображение и откат ROS по запросу) | +| `024-readme-restructure` | `7c830be` | 13 (документация, с неточностью) | +| `025-trusted-proxies-port` | `5367cb5` | 14, 16 | +| `026-split-tests` | `e1f197c` | 12 | + +## Статус замечаний + +### Закрытые ранее (п. 1–10) +Без изменений: безопасность (1–4, изменение 021), корректность (5–7, 018), производительность (8–10, 019 и 022). Регресс п. 9 контролирует +тест-линтер `tests/test_architecture.py::test_no_sync_db_calls_in_async_functions`. + +### П. 11–19 + +| № | Замечание | Приоритет | Статус | Подтверждение | +|---|---|---|---|---| +| 11 | Нет healthcheck, логирование не настроено | Низкий | ❌ Открыт | Нет `HEALTHCHECK` в `Dockerfile`/`docker-compose.yml`, нет `/healthz`; `logging` не настраивается | +| 12 | Тесты в одном файле | Низкий | ✅ 026 | 7 модулей по 68–185 строк (всего 1035), `tests/helpers.py`; тела тестов не менялись (AST совпадает), каждый модуль проходит отдельно | +| 13 | Копирование БД в режиме WAL | Средний | 🟡 Неточность | `README.md:161` советует `sqlite3 <БД> ".backup …"`; в контейнере (`python:3.12-slim`) утилиты `sqlite3` нет, том на хост не смонтирован. Рабочий способ — `sqlite3.Connection.backup` через `docker exec … python -c …` | +| 14 | За reverse-proxy блокировка видит IP прокси | Низкий | ✅ 025 | `app/security.py::client_ip` + `TRUSTED_PROXIES` (по умолчанию пусто — XFF игнорируется); на стенде поддельный `X-Forwarded-For` не повлиял на блокировку и журнал | +| 15 | `refresh=1` остаётся в адресе «Бэкапов» | Низкий | ❌ Открыт | `app/ui/routes.py:485` — `refresh_url` | +| 16 | Порт стенда не параметризован | Низкий | ✅ 025 | `"${APP_BIND:-0.0.0.0}:${APP_PORT:-8000}:8000"`; стенд на `0.0.0.0:8001` через `APP_PORT` в `.env`, override-файл не нужен | +| 17 | Откат ROS не проверен на устройстве | Средний | ✅ Подтверждён | См. «Проверка отката на устройстве» | +| 18 | Прошивка RouterBOARD после отката остаётся новее | Низкий | ⏸ Отложен | Не обрабатывается; на `5G-AC-BED` не проявилось — устройство возвращено на 7.24.4 (`fw_current` = `fw_upgrade` = 7.24.4). Актуально при долгой работе на long-term | +| 19 | Тест-линтер видит одну ступень транзитивности | Низкий | ❌ Открыт | `tests/test_architecture.py:54–56` — только `level0`/`level1` | + +## Проверка отката на устройстве (п. 17) + +Пользователь выполнил реальный откат на `5G-AC-BED` через UI (актор `ui:tstark`). Журнал событий (UTC): + +| Время | Событие | +|---|---| +| 14:26:06 | `job.created` / `job.started` — задача `ros_downgrade` | +| 14:26:07 → 14:26:10 | `backup.created` → `backup.done` — бэкап `backups/5G-AC-BED/20260928-142607.{backup,rsc}` **до** установки | +| 14:26:26 | `job.done` — «Откат 7.24.4 → 7.23.7 запущен, устройство перезагрузится» | +| 14:26:31 → 14:29:54 | `device.offline` → `device.online` — перезагрузка | +| 14:34:41 | задача `ros_update`: «**Обновление 7.23.7 → 7.24.4** запущено» — подтверждает, что откат установил 7.23.7 | +| 14:35:04 → 14:37:53 | перезагрузка, устройство снова на 7.24.4 (stable) | + +Вывод: `/system/package/update/install` в RouterOS 7 при версии канала старше установленной выполняет откат; порядок +«проверка → бэкап → установка» соблюдён; возврат штатным обновлением работает. + +## Проверка UI + +Ручная проверка интерфейса по 019–026 явно не подтверждена, но фактическая работа пользователя через UI (откат, обновление, +бэкапы, смена каналов — события `ui:tstark`) проходит без ошибок. + +## Рекомендуемый порядок + +1. **Изменение 027 — эксплуатация и хвосты:** п. 11 (`/healthz`, `HEALTHCHECK`, формат логов), п. 15 (редирект без `refresh=1`), + п. 13 (исправить совет о копировании БД в README), п. 19 (транзитивное замыкание в тест-линтере). +2. П. 18 — при необходимости долгой работы на long-term: показывать и выполнять откат прошивки к версии ROS.