From 559a56d80b50bfab1704618402037ec62899baf7 Mon Sep 17 00:00:00 2001 From: Hermes Team Date: Thu, 20 Aug 2026 22:31:07 +0700 Subject: [PATCH] docs(task): Plan A refactoring task with verified baseline statuses 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 --- ...an-a-refactoring-with-verified-baseline.md | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 agents/inbox/2026-08-21-plan-a-refactoring-with-verified-baseline.md diff --git a/agents/inbox/2026-08-21-plan-a-refactoring-with-verified-baseline.md b/agents/inbox/2026-08-21-plan-a-refactoring-with-verified-baseline.md new file mode 100644 index 0000000..95be797 --- /dev/null +++ b/agents/inbox/2026-08-21-plan-a-refactoring-with-verified-baseline.md @@ -0,0 +1,172 @@ +# Задание: 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` и не вести разработку поверх него до вердикта.