REFACTORING_REVIEW.md

Rama master · réplica de lectura del repositorio original.

# Ревью и рефакторинг Battle Forge

Ревью текущего рабочего дерева, включая существовавшие незакоммиченные изменения.
Проверены player, импорт/нормализация персонажей, операции персонажа и инвентаря,
основные пути записи боя, библиотеки DM и HTTP/Socket.IO/MongoDB-слой.
Это обзор кода и автоматических проверок, без нагрузочной проверки развёрнутой системы.

## Исправлено

- **Состояние player:** 32 разрозненных writable signals заменены шестью типизированными
  группами: session, sheet, attackState, spellState, resourceState, inventoryState.
  Сам персонаж остаётся отдельным сигналом: изменения формы не должны инвалидировать
  вычисления характеристик и оружия. Фабрики в
  [player-state.model.ts](src/app/features/player/player-state.model.ts)
  задают начальное состояние и полный сброс каждого сценария.
- **Выход и смена сессии:** сбрасываются также вкладка, оружие, режим атаки,
  раскрытая способность и ошибка импорта, которые раньше переживали logout.
  Версия сессии отделяет завершения старых запросов от текущего экрана.
  Поздний вход/импорт не восстанавливает закрытую сессию, старое применение заклинания
  не снимает pending нового запроса, ресурс не активирует эффект для другой сессии.
  Защищены success и error/finally-ветки. Уже отправленную запись это не отменяет.
- **Общие сценарии:** login/import используют startCharacterSession; варианты
  применения заклинания используют performSpellUse.
  Восемь повторов load → missing check → patch → save в CharacterService используют
  updateLoadedCharacter. Это координация записи, не транзакция.
- **Utilities:** getAbilityModifier используется парсером, нормализатором, расчётом
  атаки и player; getSpellMaxUses/getSpellUsesRemaining — UI и правилами заклинаний;
  clampInteger — числовыми полями; isPositiveInteger — расходом предметов/ресурсов.
- **Расход:** отрицательные, нулевые, дробные и нечисловые количества отклоняются
  сервисами. Раньше отрицательная quantity увеличивала количество предметов,
  а NaN мог удалить стек. removeItem больше не мутирует загруженный массив
  и отклоняет дробные/нечисловые индексы.
- **Стоимость ресурса:** фиксированная стоимость не уменьшается до остатка.
  Недостаточный ресурс блокирует кнопку и обработчик.
- **Импорт:** ошибка чтения/сохранения видна на экране, а не только в logger.
  Ошибка разбора JSON отображается и до входа.

## Оставшиеся проблемы по приоритету

### P1. Потеря обновлений при параллельной работе

[CharacterService](src/app/core/services/character.service.ts) и
[InventoryService](src/app/core/services/inventory.service.ts) читают лист, меняют
копию и заменяют весь документ. Расход предмета и обновление HP могут прочитать одну
версию; поздний saveCharacter вернёт старое HP либо старый инвентарь.
Два расхода одного ресурса могут сохранить одинаковый остаток.

[BattleService.takeDamage](src/app/core/services/battle.service.ts) вычисляет HP
из локального realtime-снимка: два удара также могут потерять один результат.
turnTransition сериализует смену хода только внутри одного экземпляра сервиса.

**Предложение:** серверные команды consumeItem/useResource/castSpell/applyDamage
с проверкой остатка и условной атомарной записью; revision и compare-and-set для
редактирования листа. Конфликт возвращать явно и обновлять клиентский снимок.
Проверять двумя конкурентными клиентами. Клиентская очередь не решает эту проблему.

### P1. Изменения нескольких документов не атомарны

[DataStoreService.update](server/src/data/data-store.service.ts) выполняет группы
операций последовательно. CharacterService.updateCharacter проверяет отсутствие
нового имени, затем записывает новое и удаляет старое. Ошибка посередине оставляет
дубликаты; параллельное создание между проверкой и записью может быть перезаписано.

replaceCollection удаляет коллекцию **до** проверки всех дочерних путей и записи новых
документов. Некорректный ключ либо ошибка записи способны оставить частичную коллекцию.
Урон игроку отдельно записывается в комнату и лист, поэтому ошибка второй записи
оставляет разные HP.

**Предложение:** валидировать batch до первой записи; использовать серверные команды
и MongoDB-транзакции для изменений нескольких документов с учётом конфигурации MongoDB.
Ввести стабильный id персонажа вместо имени-ключа; имя сделать редактируемым полем.
Миграцию провести с поддержкой прежних ссылок.

### P1 при доступе недоверенных клиентов. Вход по имени не ограничивает API

[DataController](server/src/data/data.controller.ts) предоставляет CRUD по path,
[DataGateway](server/src/data/data.gateway.ts) принимает подписку по path.
В server/src нет guards для ролей DM/player или проверки владения персонажем.
Скрытие кнопок и экран входа Angular не ограничивают API.

**Предложение:** серверная сессия и проверка роли/комнаты/персонажа для HTTP и Socket.IO.
Текущая модель подходит доверенной группе, но не разграничению доступа игрока и DM.

### P2. Realtime-снимки не имеют версии

[DataGateway](server/src/data/data.gateway.ts) присоединяет socket к комнате, затем
читает и отправляет снимок. publishRelated тоже читает асинхронно. Позднее чтение
может доставить старое состояние после нового; клиент не проверяет revision.

**Предложение:** включить монотонную revision в чтения/события и отбрасывать старые версии.
Разделить результат команды и доставку уведомления: ошибка publishRelated после
успешной записи сейчас превращается в ошибку HTTP-запроса, хотя данные уже сохранены.
Для безопасного повтора команды нужен requestId.

### P2. Player и BattleService всё ещё имеют много обязанностей

Группировка сигналов упрощает переходы UI, но не решает размер
[player.component.ts](src/app/features/player/player.component.ts) и шаблона.
BattleService совмещает I/O, историю, ходы, эффекты, спасброски и синхронизацию листа.

**Следующий этап:**
1. Выделить локальные player-компоненты sheet, arena, spells/resources и inventory-dialog.
   Передавать readonly данные и события; состояние диалога держать рядом с диалогом.
2. Управление сессией/подпиской вынести в facade в providers экрана с жизненным циклом
   компонента. Не делать пользовательское UI-состояние root singleton.
3. Выделить чистые переходы боя, возвращающие nextState и history entry.
   BattleService оставить координацию команд, серверу — атомарность.
4. Переносить виджеты в shared/ui при реальном повторном использовании.
   Не вводить универсальный CRUD/store для всех сущностей: правила нормализации различаются.

### P2. TypeScript не валидирует входящие документы

[SetDataDto / UpdateDataDto](server/src/data/data.dto.ts) проверяют оболочку запроса,
но data остаётся unknown/Record. Клиентская нормализация исправляет часть старых записей,
однако API позволяет сохранить невалидное содержимое. Парсер LSS также полагается
на объявленные типы многих полей JSON.

**Предложение:** отдельные схемы внешнего LSS, сохранённого персонажа и серверных команд.
Проверять команды на сервере, импорт — на входной границе; разделять проверку нового
ввода и исправление legacy-данных.

### P2. Зависимости требуют отдельного обновления

Проверка корневого lockfile через npm audit --json 27.09.2026 сообщает о 18 затронутых
пакетах: 17 moderate и 1 high. Это не 18 независимых уязвимостей: часть записей —
зависимые пакеты одной цепочки. Серверный lockfile этим запуском не проверялся.

High относится к fast-uri@3.1.5: npm explain показывает dev-цепочку через ajv и
Angular CLI/devkit. Поэтому результат аудита сам по себе не доказывает уязвимость
production-приложения. Пример исходного advisory:
[fast-uri / repeated hostname percent-decoding](https://github.com/advisories/GHSA-fph4-wmhf-6fwf).

Есть также замечания к Angular и Vitest. Предложенный npm способ исправления части
Vitest-цепочки требует major-обновления, а package.json закрепляет vitest в overrides.
Следует отдельно согласовать совместимые версии Angular/Analog/Vitest, проверить
применимость advisories и повторить test:all/build. Автоматический audit fix --force
в рефакторинге не применялся.

### P3. Инструкции проекта отстали от реализации

В .agents/skills/battle-forge-project-map и доменных навыках ещё указаны FirebaseService,
RTDB и emulator. В коде уже RealtimeDataService, NestJS, Socket.IO и MongoDB;
test:all запускает coverage и серверные тесты. Инструкции могут направлять работу
в несуществующие файлы.

**Предложение:** отдельно обновить карту и инструкции тестирования под текущий стек.
Этот прикладной рефакторинг не требует миграции хранилища или новых runtime-зависимостей.

## Проверка

- npm run test:all: 52 frontend-файла, 320 тестов; 6 серверных файлов, 22 теста — успешно.
- Coverage: statements/lines 87,34%, branches 75,69%, functions 80,24%; порог 70% пройден.
- npm run build: production-сборка успешна, ошибок шаблонов и бюджетов нет.
- npm run typecheck: строгая проверка приложения и тестов успешна.
  Исправлены типы DOM-запросов и неполные fixtures; @types/node добавлен только в devDependencies
  для существующего теста с node:fs. Команда typecheck включена в test:all.
- Добавлены 32 регрессионных случая: поздние ответы/выход/повторный вход, сброс UI,
  фиксированная стоимость ресурса, ошибки импорта, недопустимые количества и индексы.
- Проверка зависимостей: npm audit завершается с замечаниями, описанными выше.
- Ручная проверка браузера и конкурентные интеграционные тесты с MongoDB не выполнялись.