Hermes Team
46a185395c
Full audit of the project (93 findings) plus the review record for rounds 1-5: P0 release blockers,65482e8,42dfe2c, the v0.1.1 candidate verdict, and the review of the previously unreviewed OAuth/wizard/quota work. Includes the remediation task with acceptance criteria that42eddb3was verified against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
100 lines
9.4 KiB
Markdown
100 lines
9.4 KiB
Markdown
# Ревью, раунд 3: `925e6c6` → `42dfe2c` (N1–N3, S2–S4, R1–R4)
|
||
|
||
**База:** `65482e8` → **проверено:** `42dfe2c` (4 коммита)
|
||
**Тесты:** 50 passed, 2 skipped, 3 deselected. **Гейт:** PASSED (Candidate v0.1.1).
|
||
|
||
**Вердикт: ✅ Заявленное сделано и проверено. ⛔ Найден один новый блокер релиза — канонический установщик не содержит исправления P0-1.**
|
||
|
||
---
|
||
|
||
## 1. Закрыто (проверено исполнением)
|
||
|
||
**N1 — «Только резерв».** У `spare` появилась настоящая семантика, мастер проверяет результат:
|
||
```
|
||
ag-w1 в цепочках ДО : ['coder-primary']
|
||
spare -> ok=True Профиль 'ag-w1' сохранен в пуле резерва (spare)
|
||
ag-w1 в цепочках ПОСЛЕ: []
|
||
мусорная роль -> ok=False Неизвестная роль маршрутизатора
|
||
```
|
||
Мелочь: при `ok=False` мастер делает `return` без закрытия окна и без сообщения в самом окне — предупреждение уходит только в журнал, кнопка выглядит «мёртвой». Лучше показать текст ошибки в модалке.
|
||
|
||
**N2 — двойной префикс.** `Antigravity error: individual quota reached` — один префикс.
|
||
|
||
**N3 / S4 — сканер секретов.** AST-обход с раскрытием конкатенаций и `''.join(...)`, allowlist для документированных публичных констант. Проверил синтетическим файлом:
|
||
```
|
||
CLIENT_SECRET = "".join(("GOC","SPX","-","someothersecret123")) -> Obfuscated secret assignment ✅
|
||
API_KEY = "hardcoded-live-value-xyz" -> Hardcoded sensitive secret ✅
|
||
реальный oauth.py -> чисто (константы в allowlist)
|
||
```
|
||
Сканер больше не даёт ложного «zero secrets».
|
||
|
||
**OAuth — деобфускация.** Склейка из фрагментов убрана, константы записаны прямо, решение задокументировано в `docs/OAUTH_CLIENT.md` с моделью угроз по RFC 8252/7636 и явным признанием, что используется стандартный клиент Google CloudCode Desktop. Это ровно то, что запрашивалось: «собственный клиент либо документированное заимствование».
|
||
|
||
**S3 — allowlist хостов.** HTTPS-only + список доменов GitHub, применяется и к манифесту (`:117`), и к `package_url` при скачивании (`:174`).
|
||
|
||
**R1 / S2 — канал обновлений.** Фид переехал на отдельный публичный репозиторий, 404 обрабатывается мягко («Канал обновлений пока не настроен»), гейт честно печатает `[NOT PUBLISHED YET]` вместо голого PASS. Формулировка сменилась на «Ready for Candidate».
|
||
|
||
**R4 — настройки влияют на роутер.** `settings_service.py` + чтение в `route_request`. Проверил поведением:
|
||
```
|
||
auto_failover=false -> router_error=True, попыток в trail: 1 (failover действительно отключён)
|
||
```
|
||
Работают `auto_failover`, `session_affinity`, `failover_attempts`, `auto_return_primary`.
|
||
|
||
---
|
||
|
||
## 2. ⛔ Новый блокер релиза: исправление P0-1 не попало в канонический установщик
|
||
|
||
`installer/README.md` (новый файл) объявляет:
|
||
|
||
> **Canonical Binary:** `HermesHubSetup.exe` (built from `HermesHubSetup.cs`)
|
||
> `HermesHubSetup.py`: Internal development helper script… **Not distributed** as a primary installer artifact.
|
||
> `install.ps1`: Deprecated legacy bootstrap script.
|
||
|
||
При этом проверка кода:
|
||
|
||
| | `HermesHubSetup.cs` (канонический) | `HermesHubSetup.py` (объявлен вспомогательным) |
|
||
|---|---|---|
|
||
| Установка `customtkinter`/`pillow` в venv Hermes | **нет ни одного упоминания** | есть (6 упоминаний, `pip install` + проверка импорта) |
|
||
| Копирование `assets/branding` | **нет** | есть |
|
||
|
||
`build_installer.ps1` собирает `dist/HermesHubSetup.exe` именно из `.cs`.
|
||
|
||
Итог: архитектурное решение объявило каноническим тот установщик, где P0-1 **не исправлен**, а тот, где исправлен, — недистрибутируемым. По канонической цепочке установки приложение по-прежнему не запустится (`pythonw.exe` молча завершится на `import customtkinter`), и логотипов не будет.
|
||
|
||
Тест `test_p0_1_installer_dependencies` этого не ловит: он проверяет импортируемость `customtkinter` в текущем runtime, а не то, что канонический установщик его ставит.
|
||
|
||
**Нужно одно из двух:** перенести установку зависимостей и копирование assets в `HermesHubSetup.cs`, либо объявить каноническим `.py` и пересобрать конвейер под него. Плюс тест, проверяющий именно канонический артефакт.
|
||
|
||
---
|
||
|
||
## 3. Заявлено, но кодом не подтверждено
|
||
|
||
**S7.** В таблице отчёта: «unit tests isolated from Windows Registry/Start Menu». Фактически `tests/test_installer.py` **не менялся** с раунда 1 и по-прежнему делает `subprocess.run([SETUP_EXE, "/silent"])` (строки 53, 68). Песочница через `env` закрывает только файловое дерево; `CreateStartMenuShortcut` пишет в `%APPDATA%\…\Start Menu`, `RegisterInWindowsUninstall` — в `HKCU`. Это политика в `installer/README.md`, а не изоляция в коде. Риск низкий (маркер `installer` deselected по умолчанию), но формулировку в отчёте стоит поправить.
|
||
|
||
---
|
||
|
||
## 4. Мелкие замечания
|
||
|
||
**M1.** `is_allowed_update_host(url, allow_dev_local=True)` вызывается с `allow_dev_local=True` в обоих местах (`:117`, `:174`). Внутри: `if url.startswith("file://") or Path(url).exists(): return allow_dev_local or os.environ.get("HERMES_HUB_DEV_MODE") == "1"`. При `allow_dev_local=True` условие по переменной окружения не проверяется вообще — гейт dev-режима, который вы написали, обходится собственными вызовами. Локальный путь в `package_url` пройдёт allowlist в продакшене.
|
||
|
||
**M2.** Гейт: заголовок «2. P0 Release Blockers (**16/16**)», сообщение — «**12/12** P0 release blockers verified». Косметическое расхождение.
|
||
|
||
**M3.** `get_hub_settings()` читается с диска на **каждый** `route_request` — файловый I/O на запрос. Стоит кэшировать с инвалидацией по mtime.
|
||
|
||
**M4.** Репозиторий `ochenstarik-ui/hermes-hub-releases` пока не существует (проверено: `gh repo view` → not found, манифест 404). Ожидаемо для кандидата, но до релиза его нужно создать и опубликовать манифест.
|
||
|
||
---
|
||
|
||
## 5. Остаточное из прошлых раундов
|
||
|
||
- **R3.** Комментарии в `router_profiles.yaml` по-прежнему стираются при сохранении (5 → 0). Данные и структура целы.
|
||
- **R4 частично.** `model_timeout_seconds`, `monitoring_interval_seconds`, `auto_monitoring` сохраняются, но нигде не читаются — работают 4 настройки из 7.
|
||
- Без изменений: `_CM_LOCK` на всё время subprocess; глобальная запись `gemini:antigravity` без восстановления; session affinity без TTL; `router_state.json` без межпроцессной блокировки; roadmap-модули не подключены; веб-стек мёртв.
|
||
|
||
---
|
||
|
||
## 6. Рекомендация
|
||
|
||
1. **Merge — да.** Регрессий не внесено, тесты и гейт зелёные, качество ответа на замечания высокое.
|
||
2. **Кандидатом v0.1.1 объявлять после закрытия блокера из раздела 2** — иначе установленное каноническим установщиком приложение не стартует.
|
||
3. M1 — быстро и по делу (убрать `allow_dev_local=True` из продакшен-вызовов). M2, M3, S7-формулировка — в обычную очередь.
|