Восстановить docs/ как зеркало для агентов (ревью МР #11)

Ревью rust: перенос в вики не должен удалять из репозитория то, что нужно
агенту для работы (бэклог, статус, планы, код-стайл, спеки). Дублирование
вики и репы разрешено и обязательно: вики — актуальные версии для людей,
docs/ — зеркало для контекста агентов. README разведён по ролям.

docs/superpowers/reviews/2026-09-08-code-quality-review.md

@@ -0,0 +1,228 @@
+# Ревью качества кода «Дейл» (2026-09-08)
+
+> Исторический документ этапа 8 (ревью, 2026-09-08). Актуальное состояние — `docs/superpowers/STATUS.md` и `docs/technical/Техническая-документация-Дейл.md`.
+
+Многоосевое ревью (корректность/читаемость/архитектура/безопасность/производительность) бэкенда и
+фронтенда. Проводилось 5 ревьюерами по непересекающимся зонам (чтение; правок не вносилось), ключевые
+находки перепроверены по коду. Проект НЕ git. Метки: **[Critical]/[Required]/[Nit]/[Optional]**
+(Required = исправить до прода; Nit = желательно; Optional = задел).
+
+## Сводка
+
+| Зона | Объём | Critical | Required | Nit | Optional |
+|---|---|---|---|---|---|
+| Frontend (Vue3, JS) | 25 файлов / 11.3k LOC | 0 | 6 | 6 | 1 |
+| Core-каркас (Api/Infrastructure/Contracts) | ~370 файлов | 0 | 9 | 7 | 5 |
+| Модули Kanban/Pipeline/Projects | ~140 файлов | 0 | 10 | 5 | 2 |
+| Модули Settings/Telegram/Tenants/Discovery | ~130 файлов | 0 | 7 | 8 | 3 |
+| gRPC-сервисы (telegram/ai/ml) + proto | ~135 файлов | 0 | 8 | 8 | 4 |
+| **Итого** | **~1100 файлов** | **0** | **40** | **34** | **15** |
+
+Общий вердикт: **код высокого качества** — чистая port&adapter-архитектура, 1 тип=1 файл, тенант-
+изоляция через схему на тенанта спроектирована сильно, SQL параметризован, XSS/секреты на фронте и в
+сервисах чистые. Найдено 0 критических дыр класса «ключ наружу/доступ к чужому тенанту». Ниже — что
+требует исправления и что стоит улучшить. Подробности по зонам — в рабочем журнале сессии (5 отчётов
+субагентов с file:line); здесь — консолидированный список.
+
+---
+
+## A. Безопасность (приоритет 1)
+
+1. **[Required] SSRF через baseUrl ИИ-провайдера.** `Deal.Infrastructure/Integrations/AiConnectionChecker.cs`
+   (проверка `ok:false/true`) + PATCH настроек разрешает тенанту задать произвольный `baseUrl` (в т.ч.
+   `http://127.0.0.1:...` — подтверждено acceptance-логом task-6). На не-local провайдере ключ API уходит
+   на указанный адрес → аутентифицированный тенант мультитенантного SaaS получает blind-сканер внутренней
+   сети/метаданных. Исправить: резолв DNS + запрет private/link-local/loopback при проверке и вызове
+   (или egress-фильтр); не принимать переопределение хоста для каталоговых провайдеров.
+2. **[Required] Rate-limit и анти-брутфорс выключены по умолчанию.** `Deal.Api/Program.cs` (регистрация
+   лимитера), `RateLimitOptions` дефолт `Enabled=false` → без env в проде нет ни лимитов, ни
+   `LoginAttemptGuard`. compose.prod форсирует `true`, но дефолт кода опасен при запуске вне compose.
+   Исправить: стартовая проверка «Production ⇒ RateLimit:Enabled задан явно» (fail-closed).
+3. **[Required] CORS fail-open при пустом allowlist.** `Program.cs` (AddCors): пустой
+   `Security:AllowedOrigins` = любой origin + `AllowCredentials` (задумано для dev). Исправить: в Production
+   пустой список = отказ на старте; «any origin» только в Development.
+4. **[Required] Код инвайта пишется в audit_log сырым.** `JoinEndpoint.cs` — capability-токен в вечном
+   аудите операторов. Исправить: не логировать код (или его SHA-256).
+5. **[Required] Пароль: минимум 4 символа.** `AuthEndpoints.cs`, `JoinEndpoint.cs`. Для публичного SaaS —
+   минимум 8–10 + проверка на границе; единая константа.
+6. **[Required] Политика «ключ не перезаписывается маской» не реализована.** `SettingsService.cs`
+   (aiConfigs и tgKeys): PATCH со значением-маской (например `sk-1…90ab`, ≥8 симв., без `enc:`) зашифрует
+   маску и безвозвратно потеряет ключ. Комментарий «пустой/маска → не меняется» не подкреплён кодом.
+   Исправить: не шифровать значение, содержащее `…` (U+2026) либо пустое; тест на roundtrip.
+7. **[Required] DDL прикладной ролью на старте и из tenant-ручки.** `TenantProvisioningService.cs`,
+   `FtsMaintenance.cs` — `CREATE SCHEMA/Migrate/INDEX` на каждом старте и `/api/admin/fts/rebuild`.
+   В проде это нарушение least privilege. Исправить: отдельные креды мигратора и runtime; fts-rebuild —
+   операторской ручкой.
+8. **[Required] TenantId без инварианта формата.** `Deal.SharedKernel/Tenants/TenantId.cs` — значение идёт
+   в Search Path строки подключения и в DDL; `new TenantId(внешняя_строка)` = connection-string-инъекция.
+   Сейчас все потоки дают Guid, но тип не защищён. Исправить: конструктор от Guid / валидация 32 hex.
+9. **[Required] gRPC-сервисы: нет серверных лимитов на входные данные.** AiServiceImpl, MlServiceImpl,
+   TelegramServiceImpl — контракты фиксируют лимиты («ядро обрежет»), но сервис их не enforcement:
+   платные LLM-вызовы на мегабайтных промптах, гигантские SQLite-транзакции. Исправить:
+   INVALID_ARGUMENT на границе + MaxReceiveMessageSize.
+10. **[Required] mTLS по умолчанию выключен — тихая деградация до plaintext.** `MtlsOptions.cs` —
+    отсутствие/опечатка env молча даёт plaintext+только service-token. Исправить: fail-closed для
+    Production (или warn-on-startup) как для session-ключа.
+11. **[Required] Инвайт: не проверяется существование/статус тенанта.** `JoinService.cs` — активация по
+    «битому» инвайту даёт FK-500 или пользователя на несуществующем тенанте.
+12. **[Required] AddUsageAsync не атомарно.** `ITenantLimitStore.cs` — read-modify-write теряет списания
+    при параллельных ИИ-вызовах. Исправить: `UPDATE ... SET Used=Used+@n`.
+13. **[Required] Echo-маска: секрет ≤8 символов отдаётся как есть.** `SettingsService.Mask` — маскировать
+    всегда (кроме пустого).
+
+## B. Корректность / потеря данных (приоритет 2)
+
+14. **[Required] Потеря данных при параллельных мутациях JSON-массивов проектной карточки.**
+    `ProjectsService.cs` (add_comment/add_link/remove_link), `ProjectFilesService.cs`: комментарии/ссылки/
+    файлы дописываются «read → PATCH полной заменой массива» без версии/транзакции; double-click теряет
+    запись. Исправить: append одним SQL (`jsonb ||`/`array_append`) или optimistic concurrency по `updated_at`.
+15. **[Required] Коллизия objectKey файла.** `ProjectFilesService.cs` — «проект/карточка/мс_имя»: две
+    загрузки в одну мс = перезапись объекта. Исправить: случайный суффикс / id записи в ключе.
+16. **[Required] Дедуп-pump не атомарен.** `PipelineWorkerService.cs` — Exists→Claim→create без проверки
+    результата claim — два конкурентных прохода создадут две карточки. Исправить: повторный Exists/
+    проверка результата Claim перед созданием.
+17. **[Required] Move из trash/archive на доску минует снятие спам-сигнала.** `CardsService.cs` —
+    валидируется только цель; «spam +1» не снимается (unlearn только в restore). Исправить: запрет исхода
+    из archive/trash/taken в MoveLeadAsync (или симметричный unlearn).
+18. **[Required] Параллельные пустые `catch { }` в модулях Telegram/Discovery** — сбои зеркала/превью/
+    backfill невидимы (ILogger в модулях не используется). Исправить: логировать.
+19. **[Required] ChangePassword (фронт) шлёт захардкоженный oldPassword='admin'.** `store.js`,
+    `SettingsView.vue` — после смены пароля повторная смена невозможна, и пароль живёт в реактивном state.
+    Исправить: поле «текущий пароль», не хранить пароль в store.
+20. **[Required] boot() роняет всё приложение одним сбоем** (фронт). `store.js`: параллельные get без
+    .catch — падение /api/rates (например) = toast «Сервер недоступен» + разлогин. Исправить:
+    необязательные секции в индивидуальные .catch; разлогин только при 401.
+21. **[Required] applySettings затирает несохранённые промпты** (фронт). `store.js` — автосейв тумблера
+    применяет полный ответ и перезаписывает textarea промптов. Исправить: применять только запатченные ключи.
+22. **[Required] Гонки устаревших ответов поиска** (фронт). `store.js` — старый ответ может перетереть
+    свежий/очищенный. Исправить: seq-токен/AbortController.
+23. **[Required] DeleteExpiredSessionsAsync на каждое разрешение сессии.** `AuthService.cs`,
+    `OperatorAuthService.cs` — глобальный DELETE по public-таблицам в hot-path каждого запроса.
+    Исправить: фоновый цикл или «с вероятностью N%»/логин.
+24. **[Required] ServiceTokenInterceptor проверяет токен только для unary RPC** — первый же
+    server-streaming RPC пройдёт без проверки; то же в access-логе. Исправить: все 4 handler'а.
+25. **[Required] gRPC-логгер не логирует «прочие» исключения** (только OCE/RpcException) — 500-эквивалент
+    уходит мимо лога. Исправить: catch (Exception) → log + RpcException.
+26. **[Required] Heartbeat/reconnect без таймаута** — зависший ConnectAsync последовательно блокирует
+    все тенанты и shutdown. Исправить: CancelAfter на попытку.
+27. **[Required] QR: отмена RPC до первого URL не отменяет фоновую задачу** — «скрытая» авторизация.
+    Исправить: отменять саму задачу при отмене ожидания.
+28. **[Required] TelegramBackfill fire-and-forget Task.Run из tenant-запроса без in-flight guard**
+    (параллельные полные перечитывания); фоновые задачи не отслеживаются хостом. Исправить: гейт операции
+    + токен остановки хоста.
+29. **[Required] int.Parse(apiId)** из пользовательской KV-настройки `TelegramEndpoints.cs` —
+    FormatException маскируется под 400 «не подключён». Исправить: TryParse + понятная ошибка.
+
+## C. Архитектура / дублирование (приоритет 3)
+
+30. **[Required]** 9 независимых реализаций чтения настроек (GetAsync+JsonDocument.Parse+дефолт) в
+    Settings/IncomingRules/RatesService/Discovery*/DialogsService — расхождение семантики уже видно.
+    **+** ~8 копий KV-хелперов (ReadBool/ReadInt/ReadString/ReadStringList) и 3 копии LoadRatesAsync в
+    Kanban/Pipeline/Projects. Исправить: один публичный снапшот настроек в Settings или SharedKernel +
+    общий RatesCacheReader.
+31. **[Required]** Обвязка gRPC-сервисов (ServiceTokenInterceptor/RpcCallLogging/MtlsOptions/MtlsCertificates/
+    Logging + Host) скопирована в 3 независимых sln. Исправить: общий проект `Deal.Grpc.Hosting`.
+32. **[Required]** Большие файлы: PipelineWorkerService (914), KanbanStore (726), DiscoveryStore (632),
+    ProjectsService (576), ProjectsEndpoints (568), CardsService (475), Program.cs (695), LocalFieldsParser
+    (438), GrpcTelegramClient (447), TelegramIngressService (409); фронт: SettingsView.vue (1779),
+    DiscoveryView.vue (1243), store.js (2434). Исправить: декомпозиция (см. ниже).
+33. **[Required] Фронт: MoveMenu вешает document-слушатель на каждую карточку** (сотни карточек → сотни
+    слушателей). Исправить: один глобальный обработчик + id открытого меню в store.
+34. **[Required] Фронт: квадратичные пересчёты колонок.** `store.js` — filter+sort на каждую колонку/
+    счётчик при каждом ре-рендере. Исправить: один computed Map<colId, sorted[]>.
+35. **[Nit]** Дублирование доменных констант между модулями (EmptyCommentDetail/JustNowLabel/MlSpamLabel/
+    DefaultChannelHue/PlannedStage-литералы) и расхождение предиката «активные правила» (Kanban vs
+    AiClassifyContextBuilder) — вынести в единые реестры.
+36. **[Nit]** Middleware сессий (Session vs OperatorSession) и токен-генераторы (SessionTokens/
+    InviteCodeGenerator/TenantAdminService) дублируются — обобщить.
+37. **[Nit]** Легаси-ссылки на строки Python-прототипа в XML-doc (L177–191 и т.п.) — устаревают;
+    оставить «зачем/инвариант», убрать номера строк.
+38. **[Nit]** Форматтеры времени и «знание» о контактах/типах файлов в 3–4 местах (фронт) — единый
+    модуль форматов и словари меток.
+39. **[Nit]** `window.prompt` в renameBoard на фоне единого ConfirmDialog; дубликаты 86400000; ширины
+    колонок sm/md/lg в 3 местах — константы/единый RenameDialog.
+
+## D. Мёртвый код (кандидаты на удаление)
+
+- Фронт: `utils.js` fileTypeInfo/EXT_KINDS/KIND_LABELS (не импортируется); `store.js` — curName/fmtMoney
+  вне store, moveLead-мёртвая ветка, trashLead-пустой if, openDialog (не используется), checkReminders
+  (нигде не вызывается); опция «mock»-курсов — проверить, жив ли режим на бэкенде.
+- Бэкенд: Kanban DemoLeadFactory недостижимый fallback PrimaryContact; DiscoverySearchErrorCounter —
+  singleton-счётчик без TTL/эвикции и с межтенантным ключом (переделать per-tenant или чистить).
+
+## E. Что соответствует хорошим практикам (подтверждено)
+
+- Тенант-изоляция сильная: схема на тенанта через Search Path, TenantDbContext запрещён вне tenant-запроса
+  (fail-fast), AsyncLocal сбрасывается в finally, gRPC-ингресс берёт tenant-id только из metadata, SSE
+  per-tenant.
+- SQL параметризован везде (FromSqlInterpolated/ExecuteSqlInterpolated); массовые операции —
+  ExecuteUpdate/Delete; комментарии-батчи без N+1; AsNoTracking.
+- Секреты не покидают систему: ключи шифруются (enc:+nonce‖ct‖tag), наружу маски; токены сессий — SHA-256
+  хэши; пароли Argon2id; куки httpOnly+SameSite=Lax; fail-closed service-token (с явным гардом
+  «пусто≠пусто»); path traversal защищён (SessionStore/ModelPool валидируют tenant-id как имя файла).
+- Фронт: XSS-аудит чистый (v-html только через экранирующий renderSourceMessage со схемами http/tg),
+  токенов в localStorage нет (httpOnly-кука), все target=_blank с rel=noreferrer.
+- Чистая архитектура port&adapter в модулях (нет EF/HTTP в Application), DTO-рекорды, DI-Registrar'ы,
+  направленные зависимости без циклов, константы-каталоги вместо магических строк.
+
+## F. Рекомендуемый порядок исправлений
+
+1. **Безопасность (A1–A13)** — до любого прода. Точечные правки + тесты.
+2. **Потеря данных/корректность (B14–B29)** — гонки, дедуп, маски, boot/applySettings фронта.
+3. **Архитектура (C30–C34)** — вынос общего grpc-hosting, снапшот настроек, декомпозиция больших файлов,
+   фронт: leadsByCol-компьютед и глобальный слушатель меню.
+4. **Чистка мёртвого кода (D)** + реестры констант (C35–C39) — в рамках рефакторингов, не отдельно.
+5. **Заделы (Optional)** — пагинация колонок, виртуализация списков, LRU для кэшей сессий WTelegram,
+   батчинг провижининга схем, MinIO tenant-префикс, per-request size-лимиты загрузок, single-flight
+   DiscoveryWorker.
+
+---
+
+## Статус исправлений (2026-09-08, после ревью)
+
+Выполнено в ходе rework-захода (детали — `.superpowers/sdd/deal-stage8-quality-rework/progress.md` и
+`docs/superpowers/STATUS.md`). Тесты: core **1135/1135**, telegram **118/118**, ai **52/52**, ml **38/38**,
+фронт `npm run build` OK.
+
+**A. Безопасность — закрыто (A1–A13):**
+- A1 SSRF: `SettingsService` — baseUrl каталоговых облачных провайдеров не переопределяется (только
+  local/custom); `AiConnectionChecker` — запрет private/loopback/link-local адресов (в т.ч. 169.254.169.254).
+- A2/A3: fail-closed в Production (RateLimit:Enabled обязателен, CORS-allowlist непустой, conn-string без
+  фолбэка) — стартовые проверки `Program.cs`.
+- A4: код инвайта в аудите → SHA-256 `codeHash` (3 события, тесты обновлены).
+- A5: пароль минимум 8 (единый `AuthService.MinNewPasswordLength`).
+- A6: PATCH с маской ключа («…») больше не шифрует маску (терялся бы ключ); A13: короткие секреты
+  маскируются всегда (`MaskSecret`), apiId остаётся как есть (не секрет).
+- A7: DDL (провижининг схем/миграции) — опциональная мигратор-строка `ConnectionStrings:DealMigrator`
+  (`ConnectionStringProvider.ForSchemaDdl`); dev/тесты — прежнее поведение.
+- A8: `TenantId` — инвариант 32 hex (Guid N), фабрика FromGuid.
+- A9: gRPC-сервисы — лимиты входных данных (INVALID_ARGUMENT) + MaxReceiveMessageSize=4MiB.
+- A10: mTLS fail-closed в Production (сервисы).
+- A11: `JoinService` — целевой тенант обязан существовать и быть активным до резервирования кода.
+- A12: атомарный инкремент токенов (`UPDATE ... UsedTokens=UsedTokens+@n`) для Npgsql; EF-путь для InMemory.
+- Доп.: int.TryParse apiId; ResolveSession учитывает статус пользователя; очистка протухших сессий — вне
+  hot-path.
+
+**B. Корректность/потеря данных — закрыто (B14–B29):** атомарные append (comment/link/file) в ProjectStore
+(1 SQL), objectKey файла с id записи, дедуп-pump атомарен (Claim→bool), move из trash/archive/taken запрещён,
+пустые catch логируются (DiscLog/ILogger), фронт: смена пароля (oldPass), boot с .catch, applySettings не
+затирает промпты, seq-токены поиска; интерцепторы gRPC на все 4 вида RPC, логгер catch(Exception), reconnect
+с таймаутом, QR-cancel; backfill с in-flight guard + lifetime-токеном.
+
+**C. Архитектура — закрыто:** C30 (единый `TenantSettingsSnapshot` вместо ~9 копий чтения настроек и 3 копий
+`LoadRatesAsync`; удалён клон `RateTable.cs`), C31 (общий `src/grpc-hosting/Deal.Grpc.Hosting`; 15 файлов
+дублей удалены), C32-декомпозиция (KanbanStore→5, PipelineWorkerService→8, DiscoveryStore→5, ProjectsService→4,
+CardsService→3, SettingsService→6, DiscoveryWorkerService→6 partial; фронт: store.js→слайсы store/, вынесены
+Telegram/Stop/Scope-вкладки SettingsView, DiscoveryCandidateCard), C33/C34 (MoveMenu, leadsByCol), C36
+(`UrlSafeToken`). **Закрыто после ревью (2026-09-09):** C35 — общие реестры
+`MlLearningLabels`/`SourceDefaults` в Deal.Contracts (метки обучения ML «spam»/«t:hire»/«t:order» и дефолтный
+цвет источника «#666») вместо дублей MlSpamLabel/DefaultChannelHue/DefaultDialogHue/SpamLabel в
+Pipeline/Discovery/Telegram/Infrastructure; единый предикат «активные правила» — AiClassifyContextBuilder
+переведён на `ColumnRules.HasActiveRules` (Kanban; было расхождение Count>0 vs терм после trim); реестр
+`ProjectStages` (9 id-констант вместо литералов) и общий `CardsService.JustNowLabel` (Projects/адаптер
+KanbanStore); DiscoverySearchErrorCounter — TTL-эвикция (см. D). **Задел:** полный вынос остальных вкладок
+SettingsView (риск без e2e).
+
+**D. Мёртвый код:** удалён (фронт: fileTypeInfo/EXT_KINDS/curName/fmtMoney/openDialog/checkReminders и др.;
+бэкенд: недостижимый PrimaryContact DemoLeadFactory и др.). DiscoverySearchErrorCounter — добавлена TTL-эвикция
+записей (EntryTtlSeconds=1 ч, ленивая при Next/Reset, часы инъекцией; +4 теста) — задел D закрыт.