Hermes Team
559a56d80b
Every Round 4 finding re-checked against HEAD 63c0385 instead of being carried
forward as still-open. Four are already fixed (customtkinter collection guard,
_CM_LOCK scope, gemini:antigravity restore, session affinity TTL), two are
partial, one is obsolete (web stack moved to legacy/), and five remain open.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
172 lines
17 KiB
Markdown
172 lines
17 KiB
Markdown
# Задание: Plan A рефакторинг — с проверенной baseline
|
||
|
||
## Дата поступления
|
||
2026-08-21
|
||
|
||
## Статус проверки
|
||
Черновик задания сверен с фактическим кодом. **Проверочный HEAD: `63c0385`**, `origin/main` = `63c0385`, divergence 0, рабочее дерево чистое (кроме неотслеживаемого `COCKPIT_TOOLS_ARCHITECTURE_COMPARISON.md`).
|
||
|
||
Ниже сохранена нумерация черновика; у каждого пункта проставлен **проверенный статус**. Пункты со статусом ✅ выполнять не нужно — они уже закрыты, и повторная работа по ним будет потерей времени.
|
||
|
||
> Правило черновика «сначала проверить текущий HEAD, потом чинить» — правильное. Эта версия применяет его к самому черновику.
|
||
|
||
---
|
||
|
||
## ВАЖНО: преамбула черновика устарела
|
||
|
||
Черновик утверждает как текущее состояние:
|
||
|
||
> release gate на фактическом HEAD НЕ проходил
|
||
|
||
Это было верно для `3aae1a8` (раунд 4). На `63c0385`:
|
||
|
||
```
|
||
pytest: 149 passed, 20 skipped, 3 deselected
|
||
release gate: [RELEASE GATE: PASSED] Ready for Candidate v0.1.1
|
||
ruff check .: All checks passed
|
||
```
|
||
|
||
SHA из черновика (`8314d46`, `3aae1a8`, `0d9005f`, `249a888`) — историческая точка раунда 4, не текущее состояние. С тех пор в `main` вошли `2b8b709`, `0c511cd`, `42eddb3`, `8db5c46`, `51e5b67`, `46a1853`, `63c0385`.
|
||
|
||
---
|
||
|
||
## PHASE 0. Baseline — статусы проверены
|
||
|
||
### P0-1. customtkinter regression — ✅ ЗАКРЫТО
|
||
`tests/test_codex_opencode_wizard.py` больше не импортирует `customtkinter` на уровне модуля. Проверено: в окружении без UI-зависимостей сбор проходит, UI-тесты уходят в SKIP (`98 passed, 7 skipped`), а не в ERROR. Действий не требуется.
|
||
|
||
### P0-2. Автоматическая защита от повторения — ✅ ЗАКРЫТО
|
||
Реализовано в `8db5c46`:
|
||
- `tests/test_import_invariants.py` — падает, если тестовый модуль импортирует `customtkinter` без `pytest.importorskip`; отдельно обходит все модули пакета и ловит сломанные внутренние ссылки;
|
||
- CI-job `headless` в `.github/workflows/ci.yml` — сносит `customtkinter` и гоняет прогон, то есть инвариант держится автоматикой.
|
||
|
||
Не делать второй механизм. Если нужен ещё маркер/политика — расширять существующий.
|
||
|
||
### P0-3. Public update feed — ⛔ ОТКРЫТО (проверено сейчас)
|
||
```
|
||
manifest: HTTP 200, version=0.1.1, схема валидна
|
||
sha256: 7a57137a93ad6fba5961e5c24b5d871aed5944b037f974b6137c028a111ae756
|
||
package_url: .../releases/download/v0.1.1/hermes-hub-0.1.1.zip -> HTTP 404
|
||
```
|
||
Релизов в `ochenstarik-ui/hermes-hub-releases` нет. Пункт актуален полностью.
|
||
|
||
### P0-4. Release gate и package_url — ⚠️ ЧАСТИЧНО СДЕЛАНО
|
||
Гейт **уже** делает живую проверку и печатает:
|
||
> `[PENDING GITHUB RELEASE] Manifest is live (v0.1.1), package_url is ready for release asset upload (HTTP 404 at GitHub Releases)`
|
||
|
||
То есть вводящего в заблуждение «feed полностью рабочий» больше нет. Остаётся валидная часть требования: **трёхуровневый статус** `MANIFEST_LIVE` / `PACKAGE_LIVE` / `PACKAGE_HASH_VERIFIED` вместо одного текстового сообщения, и решение — должен ли gate падать при `PACKAGE_LIVE=false` (сейчас PASSED).
|
||
|
||
### P0-5. Атомарность публикации — ⛔ ОТКРЫТО, требование верное
|
||
Порядок `build → checksum → upload → verify → publish manifest → verify feed` — правильный. Сейчас нарушен: манифест рекламирует артефакт, которого нет.
|
||
|
||
### P0-6, P0-7, P0-8. Воспроизводимость артефактов и checksums — ⛔ ОТКРЫТО
|
||
`dist/` в `.gitignore`, в git 0 файлов, `dist/checksums.txt` содержит сумму устаревшей сборки (`7e2531…`). Требования верны без правок.
|
||
|
||
### P0-9. НЕ выпускать v0.1.1 — ✅ согласовано
|
||
Тег снят с `origin` и локально (указывал на `65482e8` — коммит без исправления P0-1 и с обфусцированным секретом). Не восстанавливать без отдельного решения.
|
||
|
||
### P0-10 / P0-11. OAuth/Wizard commits — ⚠️ ФОРМУЛИРОВКА НЕТОЧНА
|
||
Черновик утверждает:
|
||
> старый review сделал беглую проверку token leakage
|
||
|
||
Фактически раунд 5 был полным ревью именно этих коммитов и дал два блокирующих дефекта:
|
||
- фабрикация данных о квотах (17 захардкоженных значений под ярлыками `*_api`);
|
||
- поддельные device codes при недоступности провайдера.
|
||
|
||
Оба закрыты в `42eddb3` и покрыты 7 регрессионными тестами (`tests/test_data_truthfulness_and_oauth_security.py`), проверено: все 7 падают на `0c511cd` и проходят на HEAD. Беглой была только проверка на утечку токенов.
|
||
|
||
**Скорректированный объём:** не полное повторное ревью, а точечная проверка сценариев из списка P0-11, которые ревью не покрывало — listener lifecycle, duplicate callback, cancel/timeout/retry, state validation, session cleanup. И обязательно сохранить подтверждённый рабочий сценарий: Antigravity аккаунт добавляется, callback отрабатывает, AutoAgent работает.
|
||
|
||
### P0-12. OAuth и Scheduler/EventBus — ⛔ АКТУАЛЬНО
|
||
Требование «OAuth — отдельная state machine, не общий refresh scheduler» и «OAuth success → targeted update, не global rebuild» верное, к текущему коду претензий пока нет — это ограничение на будущий рефакторинг.
|
||
|
||
---
|
||
|
||
## P0-13. Остаточные замечания раунда 4 — статусы проверены
|
||
|
||
| # | Замечание | Статус на `63c0385` | Комментарий |
|
||
|---|---|---|---|
|
||
| 1 | Комментарии `router_profiles.yaml` стираются | ⛔ **OPEN** | Проверено: 5 строк → 0 после одного назначения роли |
|
||
| 2 | `model_timeout_seconds`, `monitoring_interval_seconds`, `auto_monitoring` не читаются | ⛔ **OPEN** | 0 читателей вне UI и `settings_service` |
|
||
| 3 | `test_installer.py` пишет в Start Menu / HKCU | ⛔ **OPEN** | 2 реальных запуска установщика; смягчено маркером `installer` и `addopts` |
|
||
| 4 | `_CM_LOCK` удерживается на всё время subprocess | ✅ **FIXED** в `0c511cd` | Lock снимается до `agy_generate` — см. предупреждение ниже |
|
||
| 5 | `gemini:antigravity` без восстановления | ✅ **FIXED** в `0c511cd` | `prev_cred` читается до подмены и возвращается в `finally` |
|
||
| 6 | SessionAffinity без TTL | ✅ **FIXED** | `ttl_seconds=1800`, `max_entries=1000`, `prune_expired()` |
|
||
| 7 | `router_state.json` без межпроцессной блокировки | ⚠️ **PARTIALLY FIXED** | Атомарная запись через `tempfile` + `os.replace` есть; межпроцессной блокировки нет (0 совпадений `msvcrt`/`fcntl`) |
|
||
| 8 | roadmap-модули не подключены | ⛔ **OPEN** | `capability_matrix`, `lifecycle_supervisor`, `skill_registry` — по 0 импортёров |
|
||
| 9 | web stack мёртв | 🔄 **OBSOLETE** | В `42eddb3` перенесён в `legacy/` — см. P0-21 |
|
||
|
||
> **Поправка ревьюера:** в отчёте раунда 5 пункты 4 и 5 были помечены как открытые. Это ошибка — они были исправлены в `0c511cd`. Приоритет у проверки кода, а не у прошлых записей.
|
||
|
||
### Новое предупреждение по пункту 4
|
||
Lock теперь снимается **до** запуска subprocess. Это устранило сериализацию, но создало гонку: два параллельных вызова разных Antigravity-профилей успевают перезаписать глобальную запись `gemini:antigravity`, пока чужой subprocess ещё работает. Нужен либо per-credential mutex на время жизни subprocess, либо отказ от глобальной записи в пользу изоляции только через `USERPROFILE`. **Обязателен concurrency regression test** (это же требует P0-15).
|
||
|
||
---
|
||
|
||
## P0-14 … P0-21 — уточнения
|
||
|
||
### P0-14. Session Affinity TTL — ⚠️ ЧАСТИЧНО
|
||
TTL и LRU есть, но `ttl_seconds` задаётся **дефолтом в конструкторе**, а `session_affinity_ttl_seconds` из `router_profiles.yaml` не читается. Требование черновика («TTL должен реально использовать `session_affinity_ttl_seconds`») актуально и точное.
|
||
|
||
### P0-15. `_CM_LOCK` — переформулировать
|
||
Lock больше не держится вокруг subprocess. Актуальная задача — не «сузить lock», а **закрыть гонку**, описанную выше, и покрыть её тестом.
|
||
|
||
### P0-16. Global `gemini:antigravity` — ✅ закрыто
|
||
Схема `captured → changed → operation → restored in finally` реализована. Проверить только устойчивость к timeout/cancel.
|
||
|
||
### P0-17. `router_state.json` — ⚠️ частично
|
||
Атомарность есть, межпроцессной блокировки нет. Актуальная часть требования — interprocess locking, если файл действительно делят GUI и процесс Hermes.
|
||
|
||
### P0-18, P0-19, P0-20 — ⛔ актуальны без правок
|
||
|
||
По P0-19 напоминание: сейчас три настройки показываются пользователю и сохраняются в `hub_settings.json`, но рантаймом игнорируются — это тот самый некорректный contract. Вариант Б (disable в UI) допустим и дешевле.
|
||
|
||
### P0-21. Web stack — ⚠️ ФАКТ ИЗМЕНИЛСЯ
|
||
Черновик просит «не удалять web stack автоматически как dead code». **Он уже перенесён** в `legacy/gui_server.py` и `legacy/gui_cockpit.html` в `42eddb3` — из `src/` удалён, но не потерян.
|
||
|
||
Решение принимать явно:
|
||
- **A.** вернуть в `src/` как основу backend API contract для Tauri (тогда покрыть тестами — сейчас `legacy/` исключён из проверок, и `build_team_hierarchy`, который он вызывает, только что чинился от `NameError`);
|
||
- **B.** оставить в `legacy/` как справочный материал.
|
||
|
||
В обоих случаях: `fastapi` и `uvicorn` сейчас остаются **обязательными** зависимостями в `pyproject.toml`, хотя код в `legacy/` не поставляется. Привести в соответствие с решением.
|
||
|
||
---
|
||
|
||
## P0-22, P0-23, P0-24 — принять без изменений
|
||
|
||
Это самые ценные пункты черновика: они закрывают процедурный сбой, повторившийся трижды — отчёт готовился по одному коммиту, а `main` уходил вперёд. Финальный отчёт обязан содержать `START_HEAD`, `FINAL_HEAD`, `origin/main`, `git status`, и release gate должен быть выполнен именно на `FINAL_HEAD`, без функциональных коммитов после него.
|
||
|
||
---
|
||
|
||
## Дополнительно: что уже сделано ревьюером (не переделывать)
|
||
|
||
Коммиты `8db5c46`, `51e5b67`, `46a1853`, `63c0385` уже в `main`:
|
||
|
||
1. **`deepseek_adapter` починен.** Модуль никогда не импортировался: ссылался на несуществующие `ProviderAdapter` и `ProfileConfig`, реализовывал 1 из 4 абстрактных методов. Переписан под `BaseProviderAdapter`, зарегистрирован как `"deepseek"`.
|
||
2. **Инварианты импорта** — `tests/test_import_invariants.py`.
|
||
3. **CI-job `headless`**.
|
||
4. **CI больше не красный.** `ruff check .` падал на каждом пуше (1221 находка), обрывая job до pytest и release gate — в CI они не выполнялись ни разу. Добавлен `[tool.ruff]` с набором правил, ловящих ошибки (`E9, F63, F7, F82, F811`); стилевые правила выключены до отдельного прохода.
|
||
5. **Семь реальных дефектов**, найденных этим набором и исправленных:
|
||
- `HubModal` использовался без импорта — модалка назначения роли падала с `NameError` (то есть обработчик, добавленный для закрытия давнего замечания, не мог открыться);
|
||
- три `after(0, lambda: ...(e))` со ссылкой на имя из `except`, которое Python отвязывает при выходе из блока — падал сам путь сообщения об ошибке;
|
||
- `build_team_hierarchy` падал с `NameError: is_main` при каждом вызове;
|
||
- `Any` и `Tuple` в аннотациях без импорта.
|
||
|
||
## Известный риск для PHASE 3+
|
||
`antigravity_provider` — namespace-пакет (нет `__init__.py`), и его `__path__` охватывает **два** каталога: `src/` репозитория и установленный плагин в `%LOCALAPPDATA%\hermes\plugins\antigravity-provider\src`. Импорт может подтянуть модуль из развёрнутой копии другой версии. При работе с HubSnapshot/EventBus это способно давать неповторимые расхождения — учитывать при отладке.
|
||
|
||
---
|
||
|
||
## Порядок работы
|
||
|
||
Порядок фаз из черновика (PHASE 0 → 19) принимается. Уточнение к PHASE 1: «закрыть текущие P0 release/test regressions» сводится к P0-3, P0-5, P0-6, P0-7, P0-8 (релизная инфраструктура) — тестовые регрессии уже закрыты.
|
||
|
||
## Критерии приёмки
|
||
Пункты 53–72 черновика принимаются, с поправками:
|
||
- **53, 54, 55** — уже выполнены, проверить как регрессию, а не реализовывать заново;
|
||
- **56, 57** — реализовать трёхуровневый статус и определить поведение гейта при `PACKAGE_LIVE=false`;
|
||
- **64** — переформулировать: не «lock не удерживается вокруг subprocess» (уже так), а «гонка при параллельной подмене `gemini:antigravity` закрыта и покрыта тестом»;
|
||
- **70** — web stack уже в `legacy/`; критерий — «принято и обосновано явное решение A или B», а не «не удалён».
|
||
|
||
## Релиз
|
||
`v0.1.1` не выпускать. После завершения — передать ревьюеру точный `FINAL_COMMIT_SHA` и не вести разработку поверх него до вердикта.
|