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>
104 lines
9.8 KiB
Markdown
104 lines
9.8 KiB
Markdown
# Ревью, раунд 2: `65482e8` (ответ на замечания B1–B4, S1–S9)
|
||
|
||
**База:** `7926de9` → **проверено:** `65482e8`
|
||
**Метод:** повтор тех же исполняемых проверок, что выявили дефекты в раунде 1
|
||
|
||
**Вердикт: ✅ Блокеры закрыты, коммит принимается в `main`.**
|
||
**⚠️ Релиз v0.1.1 объявлять рано** — канал обновлений не работает, а релизный гейт даёт ложно-зелёный результат по секретам.
|
||
|
||
---
|
||
|
||
## 1. Блокеры — все 4 закрыты (проверено повтором)
|
||
|
||
**B1 — не-роутерный путь.** Защита в двух местах: guard в `hermes_plugin.antigravity_llm_execution` (обе ветки) и в самом `openai_completion_object`. Повтор моей проверки:
|
||
```
|
||
choices: 1
|
||
choices[0].message.content: Antigravity error: ... individual quota reached
|
||
```
|
||
Раньше — `IndexError`. Закрыто.
|
||
|
||
**B2 — фиктивные роли.** Введён `CANONICAL_ROLE_MAP`, неизвестные роли отклоняются:
|
||
```
|
||
coder -> ok=True назначен на роль 'coder-primary'
|
||
researcher -> ok=True назначен на роль 'research'
|
||
tester -> ok=True назначен на роль 'fast'
|
||
general -> ok=True назначен на роль 'fast'
|
||
spare -> ok=False Неизвестная роль маршрутизатора: 'spare'
|
||
|
||
роли ПОСЛЕ: без изменений (мусорные роли не создаются)
|
||
цепочка coder-primary: ['ag-w1', 'codex-worker-1', 'opengo-3'] <- реально обновилась
|
||
макс. длина цепочки: 4 (было 16)
|
||
```
|
||
Закрыто.
|
||
|
||
**B3 — разрушение YAML.** Схема `load`/`save` сведена. Блок `router:` сохраняется целиком со всеми значениями (`enabled`, `default_role`, `max_failover_attempts`, `cooldown_base_seconds`, `cooldown_max_seconds`, `session_affinity_ttl_seconds`); профилей 16/16, ролей 6/6, cooldown'ы не поплыли. Закрыто по существу — см. остаточное R3.
|
||
|
||
**B4 — 429 как квота.** Порядок веток исправлен:
|
||
```
|
||
429 Too Many Requests: rate limit exceeded -> RateLimitedError rate-limited retry_delay=60s
|
||
individual quota reached, resets in 2h -> QuotaExceededError quota-exhausted cooldown=7200s
|
||
```
|
||
Закрыто (заодно S1 — длительность сброса снова парсится: 7200 вместо 1800).
|
||
|
||
---
|
||
|
||
## 2. Второстепенные — закрыто 5 из 8
|
||
|
||
| | Статус |
|
||
|---|---|
|
||
| S1 время сброса квоты | ✅ 7200 s для «resets in 2h» |
|
||
| S5 `py_compile` по `.venv` | ✅ обход ограничен `target_src` |
|
||
| S6 настройки не сохранялись | ✅ `_save_settings` читает состояние виджетов (`aff_sw.get()` и др.) |
|
||
| S8 сбор pytest падал целиком | ✅ `pytest.importorskip("customtkinter")`; прогон: **44 passed, 2 skipped, 3 deselected** |
|
||
| S9 опрос OAuth после закрытия | ✅ переопределён `destroy()` со сбросом `_polling_active` |
|
||
| **S2 канал обновлений** | ❌ не исправлено |
|
||
| **S3 подлинность пакета** | ❌ не адресовано |
|
||
| **S4 слепая проверка секретов** | ❌ не исправлено |
|
||
| S7 тест установщика меняет систему | ❌ не адресовано (риск низкий: `installer` deselected по умолчанию) |
|
||
|
||
---
|
||
|
||
## 3. Новое, найденное в этом раунде
|
||
|
||
### N1. Мастер: «Только резерв (Spare)» молча не срабатывает, а журнал рапортует успех
|
||
|
||
`add_account_wizard.py:373` предлагает вариант `spare`, строка 394 маппит его в `"spare"`, строка 401 вызывает `assign_profile_to_role(..., "spare", ...)`. После правки B2 эта роль **отклоняется** (`ok=False`), но возвращаемое значение игнорируется, и следом безусловно пишется:
|
||
|
||
```python
|
||
EventLogService.get().log("account", f"... Назначен на роль: {target_role}.", level="success")
|
||
```
|
||
|
||
Пользователь выбирает «Только резерв», видит успех — назначения не происходит. До B2 создавалась мусорная роль, теперь не делается ничего; в обоих случаях функция не работала, но раньше отказ был хотя бы виден в конфиге.
|
||
|
||
**Нужно:** либо добавить `spare` в `CANONICAL_ROLE_MAP` с осмысленной семантикой (добавление в хвост цепочки нескольких ролей), либо проверять результат и показывать пользователю отказ. `auto_assign_all` возвращаемое значение проверяет (`if ok:`) — здесь надо так же.
|
||
|
||
### N2. Двойной префикс в тексте ошибки (косметика)
|
||
|
||
`_error_completion` уже добавляет `"Antigravity error: "`, guard в `hermes_plugin`/`runtime` добавляет его повторно → `Antigravity error: Antigravity error: individual quota reached`.
|
||
|
||
### N3. Релизный гейт зелёный на непроверенных вещах
|
||
|
||
- Пункт «3. Auto-Updater & Rollback → PASS»: ни один тест в `test_updater.py` не обращается к реальному манифесту — все работают на инъецированном словаре или локальных файлах. Реальный канал при этом отдаёт **HTTP 404** (проверено). Гейт подтверждает механику, но не работоспособность обновления.
|
||
- Пункт «6. Zero Credentials & Secrets → PASS»: проверка ищет файлы (`auth.json`, `*.key`, `.env*`) и живые ключи по regex `sk-…`/`opencode-…`. `CLIENT_SECRET` в `oauth.py:29`, склеенный из фрагментов именно для обхода сканеров, не обнаруживается. Гейт будет зелёным при секрете в коде.
|
||
|
||
---
|
||
|
||
## 4. Остаточное (не блокирует merge, блокирует релиз)
|
||
|
||
**R1 (S2). Встроенный апдейтер нерабочий.** `update_manager.py:82` по-прежнему указывает на `raw.githubusercontent.com/ochenstarik-ui/hermes-hub/main/dist/update_manifest.json`. Проверено сейчас: `HTTP 404`, файл в git не отслеживается (`dist/` в `.gitignore`), репозиторий приватный. Пока не появится публичный канал (GitHub Releases API или отдельный репозиторий манифестов), «Проверить обновления» всегда будет падать.
|
||
|
||
**R2 (S3). Только целостность, без подлинности.** SHA-256 берётся из того же манифеста, что и `package_url`; `HERMES_HUB_UPDATE_URL` переопределяется переменной окружения; хост `package_url` не ограничен allowlist'ом. Для механизма, распаковывающего исполняемый код в рабочий каталог, нужна подпись пакета либо жёсткая привязка к домену релизов.
|
||
|
||
**R3 (из B3). Комментарии в YAML по-прежнему стираются:** 5 строк → 0 при первом сохранении. Данные и структура больше не теряются, но пользовательские пояснения — да. Решается либо переходом на `ruamel.yaml` с сохранением комментариев, либо явным предупреждением в документации, что файл перезаписывается приложением.
|
||
|
||
**R4.** Сохранённые настройки (`hub_settings.json`) по-прежнему никем не читаются: `failover_attempts`, `session_affinity`, `auto_failover`, `model_timeout_sec`, `monitoring_interval_min` не влияют на роутер. Переключатели теперь честно сохраняются — но эффекта не дают.
|
||
|
||
**Из аудита без изменений:** `_CM_LOCK` на всё время subprocess; глобальная запись `gemini:antigravity` без восстановления; session affinity без TTL; `router_state.json` без межпроцессной блокировки; roadmap-модули не подключены; веб-стек мёртв.
|
||
|
||
---
|
||
|
||
## 5. Рекомендация
|
||
|
||
1. **Merge `65482e8` в `main`** — блокеры закрыты, регрессий не внесено, тесты зелёные.
|
||
2. **До объявления v0.1.1:** закрыть N1 (одна строка + проверка результата), поднять реальный канал обновлений (R1) либо скрыть кнопку «Проверить обновления», научить гейт видеть склеенные секреты (N3/S4) — иначе «RELEASE GATE: PASSED» вводит в заблуждение.
|
||
3. N2, R3, R4 — в обычную очередь.
|