hermes-android/agy-work/TASK-2026-08-24-01-hermes-transport-and-lan-reachability.md

188 lines
21 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Task 01: Достижимость LAN-хоста и гонки в транспорте (hermes-android)
**Repo:** `ochenstarik-ui/hermes-android`
**Assigned to:** Antigravity (режим оркестратора, два кодера)
**Priority:** CRITICAL (основной сценарий не работает + порча данных в стриме)
**Date:** 2026-08-24
**Base SHA:** `ba5f0466f3fcb83fc2367ca61727ddb897529f88`
## Роли и протокол
| Роль | Модель | Что делает |
|---|---|---|
| Оркестратор | Antigravity | Разбивает работу, маршрутизирует, принимает результат |
| Кодер 1 | Gemini Flash 3.7 high | Реализация §Scope целиком |
| Кодер 2 | Gemini Pro high | Независимая проверка работы кодера 1 и доработка |
### Раунд 1 — кодер 1
1. Сначала пишет тесты из §Required tests и **фиксирует их падение на base SHA**. Вывод падения — в отчёт дословно.
2. Только после этого правит код по §Scope, пункт за пунктом.
3. Архитектурных решений не принимает. Где в задании выбор не сделан явно — останавливается и записывает вопрос в раздел `OPEN QUESTIONS` своего отчёта, а не выбирает молча.
4. В отчёте: base SHA, список изменённых файлов, диф по каждому пункту §Scope, фактический вывод команд §Required verification.
### Раунд 2 — кодер 2
Работу кодера 1 **не переписывает целиком**. Порядок строго такой:
1. Независимо воспроизводит падение тестов на base SHA — своим запуском, не по отчёту кодера 1.
2. Читает диф против §Definition of Done и §Anti-checklist. Каждый пункт анти-чеклиста отмечает явно: `проверено — чисто` / `нарушено — <что именно>`.
3. Найденное исправляет сам, минимальным дифом поверх работы кодера 1.
4. Пишет findings по шкале `CRITICAL/HIGH/MEDIUM/LOW`. Если нарушений нет — пишет `findings: none` и перечисляет, что именно проверялось. Пустой раздел findings считается невыполненным ревью.
Кодеру 2 запрещено: удалять или ослаблять тесты кодера 1; менять §DoD; расширять область задания; закрывать пункт формулировкой «выглядит корректно» без запуска.
### Раунд 3 — оркестратор
Принимает результат только при выполнении всех условий:
- оба отчёта содержат фактический вывод команд, а не утверждение «прошло»;
- тесты из §Required tests падают на `ba5f046` и проходят после fix'а — подтверждено обоими кодерами независимо;
- список изменённых файлов совпадает с фактическим дифом;
- ни один существующий тест не удалён и не ослаблен; если тест всё же изменён — в отчёте построчное обоснование;
- расхождения между отчётами кодеров разрешены до приёмки, а не задним числом.
При расхождении выводов кодеров приоритет у того, кто приложил вывод команды. Никакой из кодеров не закрывает пункт по собственному заявлению.
## Проблема
Аудит на `ba5f046` выявил четыре дефекта, которые блокируют базовую работу клиента. Первый ломает основной пользовательский сценарий целиком, остальные три портят поток сообщений и роняют экран чата.
### 1. Приложение физически не может соединиться с LAN-хостом
`AndroidManifest.xml:17` объявляет `android:usesCleartextTraffic="false"`, файла `network_security_config.xml` в проекте нет. При этом весь онбординг построен на LAN-хосте: `hermes-pair` по умолчанию отдаёт в QR `scheme = "http"` (`hermes-pair/src/cli.rs`, `resolve_cli_endpoint`), `PairingPreviewDialog.kt:35` сам ставит галочку `allowCleartext` для http, `HostEntity.allowCleartext` хранится в БД, а `HermesRestClient.kt:37` и `JsonRpcGatewayClient.kt:86` проверяют этот флаг.
Проверки в коде корректны — но до них дело не доходит: платформа отклоняет запрос к `http://192.168.x.x:9119` раньше, с `CLEARTEXT communication not permitted`. Пользовательский переключатель декоративен.
Зеркальная половина: у LAN-хоста сертификат самоподписанный, trust anchors не настроены, поэтому вариант с `https://` тоже не соединяется.
### 2. Слушатели закрытых WebSocket-ов правят состояние живого соединения
`JsonRpcGatewayClient.kt:107-136`: `connect()` перезаписывает `activeWebSocket`, но `WebSocketListener` предыдущего сокета остаётся зарегистрированным в OkHttp. Опоздавший `onFailure` от мёртвого сокета переводит `_connectionState` в `Failed` и обрывает `pendingRequests` уже установленного соединения, после чего `HermesHostRuntime.kt:83-88` запускает переподключение. Поле `private var activeWebSocket` (`:75`) пишется из потока диспетчера OkHttp и читается из корутин без `@Volatile`.
### 3. События могут прийти в обратном порядке
Шаблон `if (!flow.tryEmit(e)) scope.launch { flow.emit(e) }` повторён на трёх хопах:
`JsonRpcGatewayClient.kt:212-216``HermesHostRuntime.kt:60-62``HermesConnectionManager.kt:97-99`.
При переполнении буфера событие уходит в отдельную корутину; буфер за это время освобождается, следующее событие проходит `tryEmit` мгновенно и обгоняет предыдущее. Для `message.delta` это перемешанные фрагменты текста в ответе.
### 4. `JsonNull` и пустые идентификаторы превращаются в валидные значения
`GatewayEvents.kt:184-209`: `jsonPrimitive.content` на `JsonNull` возвращает строку `"null"` — поле `"session_id": null` даёт `sessionId = "null"`, по которому дальше идёт поиск в `runtimeToSessionMap`. Отсутствующий `message_id` даёт `""`, и эта строка уходит первичным ключом в Room (`UnifiedMessageEntity.id`) и ключом в `LazyColumn(key = { it.id })` (`ChatScreen.kt:241`). Два сообщения с пустым id — исключение Compose `Key was already used` и падение экрана чата.
Дополнительно `.jsonPrimitive` бросает на поле-объекте, а `JsonRpcGatewayClient.kt:217-219` глушит это пустым `catch` — кадр исчезает бесследно, во всём приложении один вызов `Log`.
## Scope
Ровно четыре пункта, в этом порядке.
**1. Достижимость хоста (`SEC-01`).**
Ввести `res/xml/network_security_config.xml` и подключить его в `<application>`. Cleartext разрешён только там, где это осознанно нужно; для остального — запрещён. Явно указать в отчёте, какой атрибут выигрывает на `minSdk 26 … targetSdk 35``android:usesCleartextTraffic` или `networkSecurityConfig`, — со ссылкой на официальную документацию Android. Без подтверждённой ссылки пункт помечается `UNVERIFIED`, а не «сделано».
Флаг `allowCleartext` из БД должен продолжать работать как второй уровень защиты в коде: разрешение на уровне платформы не отменяет проверок в `HermesRestClient.validateUrlScheme` и `JsonRpcGatewayClient.connect`.
Половину про TLS с самоподписанным сертификатом в этом задании **не делать** — она вынесена в задание 06. Здесь только cleartext-путь.
**2. Идентификация сокета (`NET-01`).**
Первой строкой каждого колбэка `WebSocketListener` — отбрасывание событий не от текущего сокета. Поле `activeWebSocket` привести к потокобезопасному виду. Предыдущий сокет отменять явно перед созданием нового. Логику реконнекта в `HermesHostRuntime` в этом пункте не трогать.
**3. Порядок событий (`NET-02`).**
Убрать шаблон `tryEmit`-с-фолбэком **во всех трёх местах сразу**. Порядок доставки должен сохраняться при переполнении буфера. Допустимые решения — последовательный `Channel` с одним потребителем на хоп либо единственный эмиттер в одной корутине с `onBufferOverflow = SUSPEND`; выбранный вариант обосновать. Смешивать `tryEmit` и `emit` для одного потока нельзя.
Проверить, что при этом не появилось блокировки потока OkHttp: `onMessage` не должен suspend'иться на переполненном буфере.
**4. Валидация входящих событий (`NET-03`, `NET-04`).**
Ввести безопасное чтение примитивов: `JsonNull` и не-примитив дают `null`, а не строку `"null"` и не исключение. Событие без обязательного идентификатора (`message_id` для message/thinking/reasoning, `tool_id` для tool, `request_id` для approval/clarify/sudo/secret) отбрасывается на границе парсера и **не доходит** до репозитория ни одним путём — включая ветки `MessageDeltaEvent` и `MessageCompleteEvent` в `UnifiedSessionRepository`, которые создают сообщение сами.
Пустой `catch` в `handleIncomingMessage` заменить на обработку с логированием и счётчиком отброшенных кадров. Логгер не должен печатать значения токенов, тикетов, паролей и содержимого сообщений.
## Do not change
- Контракт протокола: имена JSON-RPC методов, форму payload сопряжения, схему `hermes://pair` — только отдельным заданием.
- Схему Room, `fallbackToDestructiveMigration`, порядок сообщений — задание 02.
- ViewModel, навигацию, камеру, экраны — задание 03.
- Мёртвый слой `feature/connections`, `feature/sessions`, `HermesGatewayRepository`, `ConnectionRepository` — задание 08. Здесь его не удалять и не чинить.
- `hermes-pair/**` — задание 07.
- Версии зависимостей, AGP, Kotlin, Compose BOM.
- Тесты, не относящиеся к §Required tests, — не переписывать.
## Anti-checklist
Кодер 2 обязан пройти каждый пункт и отметить результат явно. Это перечень способов сдать задание формально невыполненным при зелёных тестах.
1. Проверка `webSocket !== activeWebSocket` добавлена, но поле осталось обычным `var` — гонка сохранилась, тест зелёный случайно.
2. `tryEmit`-фолбэк убран в `JsonRpcGatewayClient`, но остался в `HermesHostRuntime` и/или `HermesConnectionManager`. Проверить все три файла поимённо.
3. Порядок событий «починен» переводом на `emit` прямо в `onMessage` — поток OkHttp теперь блокируется на переполненном буфере. Это регресс, а не фикс.
4. Валидация id добавлена в `GatewayEvent.parse`, но пустой id всё ещё попадает в Room и в ключ `LazyColumn` по другому пути. Проверить оба обработчика в репозитории, а не только парсер.
5. Тест написан на фейке, который не воспроизводит дефект (как `FakeUnifiedSessionDao`, скрывающий неопределённый порядок Room). Требование: каждый тест из §Required tests обязан падать на `ba5f046`. Не падает — тест не годится.
6. `network_security_config.xml` добавлен, но `android:usesCleartextTraffic="false"` в манифесте оставлен без разбора, какой из них применяется. Противоречивая пара — не «сделано».
7. В отчёте написано `green` / `PASS` для команды, которая фактически не запускалась из-за отсутствия JDK или Android SDK. Это прямое нарушение `AGENTS.md §3`.
8. Пустой `catch` заменён на логирование, которое печатает тело кадра целиком — то есть в logcat уезжают тикеты и содержимое переписки.
## Definition of Done
- Запрос к хосту с `allowCleartext = true` и адресом `http://<lan-ip>:<port>` доходит до сети; запрос к хосту с `allowCleartext = false` по-прежнему отклоняется кодом клиента.
- Опоздавший колбэк закрытого сокета не меняет `connectionState` и не обрывает `pendingRequests` актуального соединения.
- Порядок событий на выходе `HermesConnectionManager.allEvents` совпадает с порядком на входе `handleIncomingMessage` при переполнении буфера на любом из трёх хопов.
- Событие с `"session_id": null` не даёт `sessionId == "null"`. Событие без обязательного id не порождает ни строки в Room, ни элемента списка.
- Два подряд события без `message_id` не приводят к дублю ключа в `LazyColumn`.
- Отброшенные кадры считаются и логируются без утечки секретов.
- Все существующие тесты зелёные, ни один не удалён и не ослаблен.
## Required tests
Новые тесты — рядом с существующими, в `app/src/test/java/app/hermes/mobile/`.
`core/network/StaleSocketIsolationTest.kt`
- колбэк `onFailure` от предыдущего сокета после успешного переподключения не переводит состояние в `Failed`;
- `pendingRequests` активного соединения не обрываются мёртвым сокетом.
`core/network/EventOrderingTest.kt`
- 500 последовательных `message.delta` при `extraBufferCapacity`, заведомо меньшем нагрузки, приходят подписчику в исходном порядке — на всех трёх хопах;
- склеенный из дельт текст совпадает с исходным побайтово.
`core/model/GatewayEventValidationTest.kt`
- `"session_id": null``sessionId == null`, не `"null"`;
- поле-объект вместо строки не бросает и не роняет разбор кадра;
- события без `message_id` / `tool_id` / `request_id` отбрасываются;
- корректное событие после отброшенного разбирается нормально (парсер не «залипает»).
`core/repository/EmptyIdRejectionTest.kt`
- событие без `message_id`, пропущенное через `UnifiedSessionRepository`, не создаёт сообщения ни через `MessageStart`, ни через `MessageDelta`, ни через `MessageComplete`.
Каждый из этих тестов обязан падать на `ba5f0466f3fcb83fc2367ca61727ddb897529f88` и проходить после fix'а. Оба состояния проверяются и указываются в отчёте обоими кодерами независимо.
Для пункта 1 юнит-тест невозможен. Проверка — на устройстве или эмуляторе: подключение к реальному `hermes serve --host 0.0.0.0` по http, с приложением логом `adb logcat`. Если устройство недоступно, пункт помечается `UNVERIFIED` с указанием причины; писать «работает» на основании чтения кода запрещено.
## Required verification
```text
./gradlew --no-daemon testDebugUnitTest
./gradlew --no-daemon lint
./gradlew --no-daemon assembleDebug
```
Требуется JDK 17 и Android SDK с `compileSdk 35`. Если среда не позволяет выполнить команду — указать это явно вместе с текстом ошибки. Формулировки `green`, `PASS`, `готово`, `закрыто` без фактического запуска не допускаются (`AGENTS.md §3`).
Отдельно приложить вывод `git diff --stat` между base SHA и результатом.
## Result
`agents/antigravity/done/TASK-2026-08-24-01-hermes-transport-and-lan-reachability.md`
Один файл, три раздела с явными заголовками: `## Кодер 1`, `## Кодер 2 (review + доработка)`, `## Вердикт оркестратора`. Обязательное содержимое каждого — по `AGENTS.md §4`: base SHA, resulting SHA, изменённые файлы, фактически выполненные команды и их вывод, findings по шкале, список `UNVERIFIED`, оставшиеся blockers и tech debt.
CRITICAL не считается закрытым по заявлению исполнителя: нужен независимый re-review с указанием fix SHA.
## Очередь следующих заданий
Не выполнять в рамках этого задания. Порядок определён зависимостями, полный разбор — в отчёте аудита.
| № | Тема | Ключевые находки |
|---|---|---|
| 02 | Целостность хранения и таймлайна | DATA-01, 02, 03, 04, 06, 08 · BUILD-04 |
| 03 | Критичный UX | UI-01, 02, 03, 04, 06 · DATA-05 |
| 04 | CI и проверяемость тестов | BUILD-03, 05 · TEST-01, 02, 03, 04 |
| 05 | Гигиена репозитория | BUILD-01, 06, 07, 08 |
| 06 | Авторизация, TLS, секреты | SEC-02, 03, 04, 05, 06, 07, 08 · NET-08 |
| 07 | Протокол сопряжения и hermes-pair | PAIR-01…07 · SEC-09, 10 |
| 08 | Жизненный цикл соединения и фон | NET-05, 06, 07, 09 · DATA-11, 12 · UI-11 |
| 09 | Производительность и атрибуция | DATA-07, 09, 10 · UI-07 |
| 10 | Чистка мёртвого слоя и релиз | DEAD-01, 02 · BUILD-02 · UI-05, 08, 09, 10 |