Проход /code-review high и /simplify по всему проекту (охват выбран владельцем: не только диапазон этапа, а backend/app, frontend/src, тесты и инфраструктура). Ревью шло тремя прогонами по слоям, /simplify — четырьмя углами (переиспользование, упрощение, эффективность, высота исправления).
Безопасность и корректность (бэкенд)
Обход каталога ачивок.DELETE /api/admin/achievements/%2E%2E уходил в shutil.rmtree по неотвалидированному slug и сносил родительскую папку каталога — в проде это /data, то есть БД, uploads и ачивки разом. Формат slug теперь проверяется в единственной точке построения пути.
Оптимистичная блокировка не работала для правки участников и вложений: onupdate срабатывает лишь при реальном UPDATE строки matches, а эти правки её не трогают — версия не менялась, и вторая правка со старой версией проходила вместо 409.
Удаление группы с партиями упиралось в RESTRICT и вылетало голым 500 без конверта ошибки; админское удаление партии оставляло файлы вложений на томе навсегда.
Аватар: повторная загрузка с тем же расширением не двигала кэш-бастер (браузер до часа показывал старую картинку), а сама версия считалась из наивного времени как из локального и расходилась с лидербордом, где то же поле считает SQL.
Единственный владелец группы мог разжаловать сам себя — группа оставалась без владельца навсегда.
mark_read не слал SSE-сигнал (бейдж непрочитанных висел на других устройствах); created_at после правки пользователя отдавался без смещения и дата «создан» прыгала на часовой пояс.
Корректность (фронтенд)
Гонка в создании партии: барабан рандома, таймеры и строки адресовались по индексу, а удаление строки индексы сдвигает — фракция могла записаться соседу или потеряться. Строки получили стабильный id.
Разлогин при обрыве связи: транспортная ошибка не отличалась от «нет сессии», и мигание сети выкидывало пользователя на страницу входа.
Ложное «Сохранено» при неудачном переключении «активен»; недостающая инвалидация истории и профилей; мёртвый fallback дат (в интерфейс попадало NaN.NaN NaN:NaN); залипающий блок в PlaceEditor при отмене жеста; затирание несохранённого ввода в админке фракций; navigate(-1) из партии, открытой по прямой ссылке; NaN в id партии.
Упрощение и переиспользование
Три источника «текущего времени» сведены к timeutil.utcnow (инвариант из CLAUDE.md), ручные бампы версии партии — в match_service.touch.
Убраны N+1: участники страницы партий и ники пригласивших берутся одним запросом (было 20 лишних запросов с двумя join на страницу); счётчики считает СУБД вместо выгрузки строк ради len().
client_ip вместо инлайна в 19 местах, read_capped_image вместо четырёх копий чтения загруженной картинки, group_service.get_membership вместо четырёх копий запроса членства.
На фронте: lib/upload.ts вместо четырёх копий загрузки файла, readCookie/resizeImage в одном экземпляре вместо четырёх, matchAffectedKeys вместо ключей строками мимо реестра, authProbeRetry — и вместе с ним закрыт пропуск: RequireAdmin тоже перестал считать обрыв связи разлогином.
ToastContext больше не перерисовывает всех потребителей на каждый тост; PlaceEditor не дёргает DOM на каждый pointermove.
Тесты
Добавлено 7 тестов (74 → 81). Отдельно отмечу: прогон ревью по тестам показал, что моя же первая версия проверки обхода каталога была вакуумной — httpx нормализует .. в URL до отправки, запрос не доходил до обработчика, и тест был бы зелёным и без защиты. Исправлено на percent-кодированную форму с проверкой конверта ошибки; убедился, что тест краснеет при снятой защите.
Коммиты
9d9e4a3 Ревью: безопасность и корректность в сервисах бэкенда
62d75ea Ревью: тесты-регрессии на найденные дефекты
c4b72cc Ревью: корректность фронтенда
1831a3e Ревью: закрыть дыры в собственном покрытии
8ac2757 Simplify: единый источник времени, версии партии и запросов
94e2c09 Simplify: общие хелперы в роутерах
a9de68f Simplify: общий загрузчик файлов и единые ключи на фронте
Проверки
pytest — 81 passed (было 74)
npm run build (tsc --noEmit + vite build) — успешно, бандл 350.80 kB против 352.59 kB до правок
API не менялся, поэтому schema.d.ts не перегенерировался
Вынесено в отдельные задачи
Находки, которые требуют вашего решения или отдельной работы, а не молчаливой правки:
#28 — правка идущей партии проставляет места, не завершая её
#29 — историю нельзя править после смены состава группы или набора дополнений
#30 — история игр в профиле обрезается на 20 партиях без подсказки
#32 — главная четырежды пересчитывает тяжёлый CTE статистики
#33 — SSE-событие партии заставляет перезапрашивать данные всю группу
Что осознанно не менял
Версия аватара из updated_at, а не из mtime файла. Вариант с mtime убрал бы ручной бамп, но заставил бы лидерборд читать файловую систему на каждую строку — хуже, чем есть.
Формула League Points и MIN_GAMES — предмет вехи v1.5 (#22/#23).
Мелкие дубли (try: unlink except OSError в четырёх местах, две похожие функции выбора занятых игроков/фракций) — заводить общий модуль ради трёх строк дороже самого дубля.
## Что сделано
Проход `/code-review high` и `/simplify` по всему проекту (охват выбран владельцем: не только диапазон этапа, а `backend/app`, `frontend/src`, тесты и инфраструктура). Ревью шло тремя прогонами по слоям, `/simplify` — четырьмя углами (переиспользование, упрощение, эффективность, высота исправления).
### Безопасность и корректность (бэкенд)
- **Обход каталога ачивок.** `DELETE /api/admin/achievements/%2E%2E` уходил в `shutil.rmtree` по неотвалидированному slug и сносил родительскую папку каталога — в проде это `/data`, то есть БД, uploads и ачивки разом. Формат slug теперь проверяется в единственной точке построения пути.
- **Оптимистичная блокировка не работала** для правки участников и вложений: `onupdate` срабатывает лишь при реальном UPDATE строки `matches`, а эти правки её не трогают — версия не менялась, и вторая правка со старой версией проходила вместо 409.
- **Удаление группы с партиями** упиралось в `RESTRICT` и вылетало голым 500 без конверта ошибки; **админское удаление партии** оставляло файлы вложений на томе навсегда.
- **Аватар:** повторная загрузка с тем же расширением не двигала кэш-бастер (браузер до часа показывал старую картинку), а сама версия считалась из наивного времени как из локального и расходилась с лидербордом, где то же поле считает SQL.
- **Единственный владелец группы** мог разжаловать сам себя — группа оставалась без владельца навсегда.
- `mark_read` не слал SSE-сигнал (бейдж непрочитанных висел на других устройствах); `created_at` после правки пользователя отдавался без смещения и дата «создан» прыгала на часовой пояс.
### Корректность (фронтенд)
- **Гонка в создании партии:** барабан рандома, таймеры и строки адресовались по индексу, а удаление строки индексы сдвигает — фракция могла записаться соседу или потеряться. Строки получили стабильный id.
- **Разлогин при обрыве связи:** транспортная ошибка не отличалась от «нет сессии», и мигание сети выкидывало пользователя на страницу входа.
- Ложное «Сохранено» при неудачном переключении «активен»; недостающая инвалидация истории и профилей; мёртвый fallback дат (в интерфейс попадало `NaN.NaN NaN:NaN`); залипающий блок в `PlaceEditor` при отмене жеста; затирание несохранённого ввода в админке фракций; `navigate(-1)` из партии, открытой по прямой ссылке; `NaN` в id партии.
### Упрощение и переиспользование
- Три источника «текущего времени» сведены к `timeutil.utcnow` (инвариант из CLAUDE.md), ручные бампы версии партии — в `match_service.touch`.
- Убраны N+1: участники страницы партий и ники пригласивших берутся одним запросом (было 20 лишних запросов с двумя join на страницу); счётчики считает СУБД вместо выгрузки строк ради `len()`.
- `client_ip` вместо инлайна в 19 местах, `read_capped_image` вместо четырёх копий чтения загруженной картинки, `group_service.get_membership` вместо четырёх копий запроса членства.
- На фронте: `lib/upload.ts` вместо четырёх копий загрузки файла, `readCookie`/`resizeImage` в одном экземпляре вместо четырёх, `matchAffectedKeys` вместо ключей строками мимо реестра, `authProbeRetry` — и вместе с ним закрыт пропуск: `RequireAdmin` тоже перестал считать обрыв связи разлогином.
- `ToastContext` больше не перерисовывает всех потребителей на каждый тост; `PlaceEditor` не дёргает DOM на каждый `pointermove`.
### Тесты
Добавлено 7 тестов (74 → 81). Отдельно отмечу: прогон ревью по тестам показал, что моя же первая версия проверки обхода каталога была **вакуумной** — httpx нормализует `..` в URL до отправки, запрос не доходил до обработчика, и тест был бы зелёным и без защиты. Исправлено на percent-кодированную форму с проверкой конверта ошибки; убедился, что тест краснеет при снятой защите.
## Коммиты
- `9d9e4a3` Ревью: безопасность и корректность в сервисах бэкенда
- `62d75ea` Ревью: тесты-регрессии на найденные дефекты
- `c4b72cc` Ревью: корректность фронтенда
- `1831a3e` Ревью: закрыть дыры в собственном покрытии
- `8ac2757` Simplify: единый источник времени, версии партии и запросов
- `94e2c09` Simplify: общие хелперы в роутерах
- `a9de68f` Simplify: общий загрузчик файлов и единые ключи на фронте
## Проверки
- `pytest` — 81 passed (было 74)
- `npm run build` (tsc --noEmit + vite build) — успешно, бандл 350.80 kB против 352.59 kB до правок
- API не менялся, поэтому `schema.d.ts` не перегенерировался
## Вынесено в отдельные задачи
Находки, которые требуют вашего решения или отдельной работы, а не молчаливой правки:
- #28 — правка идущей партии проставляет места, не завершая её
- #29 — историю нельзя править после смены состава группы или набора дополнений
- #30 — история игр в профиле обрезается на 20 партиях без подсказки
- #32 — главная четырежды пересчитывает тяжёлый CTE статистики
- #33 — SSE-событие партии заставляет перезапрашивать данные всю группу
## Что осознанно не менял
- **Версия аватара из `updated_at`, а не из mtime файла.** Вариант с mtime убрал бы ручной бамп, но заставил бы лидерборд читать файловую систему на каждую строку — хуже, чем есть.
- **Формула League Points и `MIN_GAMES`** — предмет вехи v1.5 (#22/#23).
- Мелкие дубли (`try: unlink except OSError` в четырёх местах, две похожие функции выбора занятых игроков/фракций) — заводить общий модуль ради трёх строк дороже самого дубля.
## Отклонения от плана
Нет. Мёрж `dev→main` — релиз, он за вами.
Closes #8
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
Находки прохода /code-review high по backend/app:
- achievement_service: slug из URL шёл в путь без проверки, из-за чего
DELETE /api/admin/achievements/%2E%2E удалял rmtree'ом родительскую папку
каталога ачивок (в проде это /data — БД, uploads, ачивки целиком).
- match_service/attachment_service: версия партии = updated_at, но onupdate
срабатывает лишь при реальном UPDATE строки matches. Правка одних участников
и работа с вложениями его не вызывали, и оптимистичная блокировка молча
пропускала конкурентную запись — бампаем updated_at явно.
- admin_service: удаление группы с партиями упиралось в RESTRICT и уходило
наружу голым 500; теперь понятная ошибка. Админское удаление партии не
чистило файлы вложений с тома — они оставались навсегда.
- user_service: при повторной загрузке аватара с тем же расширением avatar_path
не менялся, updated_at не двигался, и кэш-бастер оставлял старую картинку до
часа. Плюс версия считалась из наивного времени как из локального и
разъезжалась с лидербордом, где то же поле считает SQL.
- membership_service: единственный владелец мог разжаловать сам себя и группа
оставалась без владельца навсегда.
- notification_service: mark_read не слал SSE-сигнал, и бейдж непрочитанных на
других устройствах висел до перезагрузки.
- routers/admin: created_at после правки пользователя отдавался без смещения,
и дата «создан» прыгала на часовой пояс до следующего обновления списка.
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
Четыре теста закрывают то, что чинил предыдущий коммит: обход каталога
ачивок через slug, неподвижная версия партии при правке участников,
удаление группы с партиями (409 вместо 500) и разжалование последнего
владельца.
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
Находки прохода /code-review high по frontend/src:
- CreateMatchPage: барабан рандома, таймеры и строки адресовались по индексу,
а удаление строки индексы сдвигает — фракция могла записаться соседу или
потеряться. Строки получили стабильный id.
- AdminAccountsPage: переключение «активен» глотало ошибку и всё равно
показывало «Сохранено».
- useServerEvents/useFinishMatch: завершение чужой партии не инвалидировало
историю игр и публичные профили — открытый профиль показывал состав до
завершения.
- guards/useMe: обрыв связи не отличался от «нет сессии», и мигание сети
выкидывало авторизованного пользователя на /login. Транспортные ошибки
повторяем, гварды показывают сообщение вместо редиректа.
- format: fallback на нераспознанную дату не работал (new Date не бросает
исключение), и в интерфейс попадало «NaN.NaN NaN:NaN».
- PlaceEditor: без onPointerCancel прерванный перенос оставлял блок с классом
dragging и сдвигом, которые React не снимает — они выставлены в обход него.
- AdminFactionsPage: refetch после сохранения одной фракции затирал
несохранённый ввод в остальных.
- MatchDetailPage: nav(-1) после удаления уводил из приложения при открытии
партии по прямой ссылке; id из URL мог быть NaN и уходил в запрос.
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
Прогон /code-review по тестам показал, что проверка обхода каталога ачивок
ничего не проверяла: httpx нормализует «..» в URL до отправки, запрос уходил
на /api/admin/ и до обработчика не доходил — тест был бы зелёным и без
защиты. Теперь percent-кодированная форма плюс проверка конверта ошибки,
чтобы промах роутинга не выдавал себя за отказ.
Добавлено недостающее: передача владения группой (обратная сторона защиты
последнего владельца), сдвиг версии партии при загрузке вложения, совпадение
кэш-бастера аватара между профилем и лидербордом. Проверка выживания группы
после отказа в удалении теперь смотрит на саму группу и её партии, а не
только на код ответа.
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Fk74jkkszahEHSjBzTjD
Проход /simplify по backend/app:
- Три источника «текущего времени» (_utcnow в models.py и match_service.py при
живом timeutil.utcnow) сведены к одному — это прямо инвариант из CLAUDE.md.
- Бамп версии партии из двух независимых мест собран в match_service.touch:
следующая точка мутации, не трогающая строку matches, теперь имеет очевидный
способ сделать правильно.
- membership_service переиспользует group_service.get_membership вместо трёх
копий одного запроса; защита последнего владельца — один хелпер на удаление
и смену роли вместо двух похожих блоков.
- Убраны N+1: участники страницы партий и ники пригласивших берутся одним
запросом вместо запроса на строку (20 партий = 20 лишних запросов с двумя
join каждый).
- Счётчики партий считает СУБД (COUNT/MAX) вместо выгрузки всех строк ради
len() и max() в Python.
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
- IP клиента для аудита брался инлайном в 19 местах шести модулей; теперь
security.client_ip — за привратником адрес придётся читать из
X-Forwarded-For, и одна точка правки для этого обязательна.
- Чтение загруженной картинки (лимит размера + sniff формата) было скопировано
в четыре обработчика; вынесено в user_service.read_capped_image.
- Лимит размера вложения жил двумя одинаковыми константами в игроцком и
админском роутере — перенесён к самим вложениям.
- update_nickname и set_active_group переиспользуют nickname_format_ok и
group_service.get_membership вместо собственных копий проверки.
- Убраны осиротевшие импорты и комментарий-заготовка о вложениях, которые
давно реализованы (MatchAttachment).
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
- Четыре копии «ресайз → FormData → CSRF из cookie → fetch → разбор конверта
ошибки» (аватар, фото партии, фото из админки, иконка ачивки) сведены в
lib/upload.ts. Иконка по-прежнему уходит оригиналом: ресайз в JPEG убил бы
прозрачность герба.
- Три копии readCsrfToken и вторая копия resizeImage удалены — берём readCookie
из api/client.ts и resizeImage из lib/image.ts с параметром размера.
- Ключи, протухающие от партии, собраны в matchAffectedKeys: раньше они были
написаны строками мимо реестра qk в двух местах, и переименование ключа
сломало бы инвалидацию молча.
- Различение «нет сессии» и «нет связи» вынесено в authProbeRetry и применено к
обеим пробам. Прошлый заход чинил только игроцкие гварды, и RequireAdmin
по-прежнему выкидывал админа на страницу входа при обрыве связи.
- ToastContext больше не пересоздаёт значение контекста: провайдер обёрнут
вокруг всего приложения, и каждый тост перерисовывал всех потребителей.
- PlaceEditor не трогает DOM, пока цель подсветки не изменилась (было
querySelectorAll на каждый pointermove).
#8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Что сделано
Проход
/code-review highи/simplifyпо всему проекту (охват выбран владельцем: не только диапазон этапа, аbackend/app,frontend/src, тесты и инфраструктура). Ревью шло тремя прогонами по слоям,/simplify— четырьмя углами (переиспользование, упрощение, эффективность, высота исправления).Безопасность и корректность (бэкенд)
DELETE /api/admin/achievements/%2E%2Eуходил вshutil.rmtreeпо неотвалидированному slug и сносил родительскую папку каталога — в проде это/data, то есть БД, uploads и ачивки разом. Формат slug теперь проверяется в единственной точке построения пути.onupdateсрабатывает лишь при реальном UPDATE строкиmatches, а эти правки её не трогают — версия не менялась, и вторая правка со старой версией проходила вместо 409.RESTRICTи вылетало голым 500 без конверта ошибки; админское удаление партии оставляло файлы вложений на томе навсегда.mark_readне слал SSE-сигнал (бейдж непрочитанных висел на других устройствах);created_atпосле правки пользователя отдавался без смещения и дата «создан» прыгала на часовой пояс.Корректность (фронтенд)
NaN.NaN NaN:NaN); залипающий блок вPlaceEditorпри отмене жеста; затирание несохранённого ввода в админке фракций;navigate(-1)из партии, открытой по прямой ссылке;NaNв id партии.Упрощение и переиспользование
timeutil.utcnow(инвариант из CLAUDE.md), ручные бампы версии партии — вmatch_service.touch.len().client_ipвместо инлайна в 19 местах,read_capped_imageвместо четырёх копий чтения загруженной картинки,group_service.get_membershipвместо четырёх копий запроса членства.lib/upload.tsвместо четырёх копий загрузки файла,readCookie/resizeImageв одном экземпляре вместо четырёх,matchAffectedKeysвместо ключей строками мимо реестра,authProbeRetry— и вместе с ним закрыт пропуск:RequireAdminтоже перестал считать обрыв связи разлогином.ToastContextбольше не перерисовывает всех потребителей на каждый тост;PlaceEditorне дёргает DOM на каждыйpointermove.Тесты
Добавлено 7 тестов (74 → 81). Отдельно отмечу: прогон ревью по тестам показал, что моя же первая версия проверки обхода каталога была вакуумной — httpx нормализует
..в URL до отправки, запрос не доходил до обработчика, и тест был бы зелёным и без защиты. Исправлено на percent-кодированную форму с проверкой конверта ошибки; убедился, что тест краснеет при снятой защите.Коммиты
9d9e4a3Ревью: безопасность и корректность в сервисах бэкенда62d75eaРевью: тесты-регрессии на найденные дефектыc4b72ccРевью: корректность фронтенда1831a3eРевью: закрыть дыры в собственном покрытии8ac2757Simplify: единый источник времени, версии партии и запросов94e2c09Simplify: общие хелперы в роутерахa9de68fSimplify: общий загрузчик файлов и единые ключи на фронтеПроверки
pytest— 81 passed (было 74)npm run build(tsc --noEmit + vite build) — успешно, бандл 350.80 kB против 352.59 kB до правокschema.d.tsне перегенерировалсяВынесено в отдельные задачи
Находки, которые требуют вашего решения или отдельной работы, а не молчаливой правки:
Что осознанно не менял
updated_at, а не из mtime файла. Вариант с mtime убрал бы ручной бамп, но заставил бы лидерборд читать файловую систему на каждую строку — хуже, чем есть.MIN_GAMES— предмет вехи v1.5 (#22/#23).try: unlink except OSErrorв четырёх местах, две похожие функции выбора занятых игроков/фракций) — заводить общий модуль ради трёх строк дороже самого дубля.Отклонения от плана
Нет. Мёрж
dev→main— релиз, он за вами.Closes #8
🤖 Generated with Claude Code
https://claude.ai/code/session_01BoiJK9ux8peeyjLb8TYjFf