hermes-hub/agents/inbox/2026-08-20-review-round3-42dfe2c.md
Hermes Team 46a185395c docs(review): add independent audit, five review rounds, and remediation task
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 that 42eddb3 was verified against.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-20 22:22:57 +07:00

9.4 KiB
Raw Blame History

Ревью, раунд 3: 925e6c642dfe2c (N1N3, S2S4, R1R4)

База: 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-формулировка — в обычную очередь.