23eb3f5233
Задания, отчёты и патчи, лежавшие в C:\Users\Ochenstarik\projects и в домашней папке, перенесены в agents/. Разложено по агентам там, где имя файла позволяло определить автора; остальное — в _salvage-2026-08-18/ и разбирается вручную. Патчи в notes/salvage-2026-08-18/ — незакоммиченная работа из брошенных рабочих копий: она существовала только на диске. Тяжёлое (релизные архивы, инсталляторы, наборы данных) в репозиторий не попало: оно лежит рядом, в Agent_projects/_archive и Agent_projects/_data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
204 lines
20 KiB
Markdown
204 lines
20 KiB
Markdown
# ТЗ: B-3R — репарация Block B-3 до пригодного к merge состояния
|
||
|
||
Репозиторий реализации: `C:\Users\Ochenstarik\projects\server-monitor-manager-task3-b3`
|
||
Ветка: `hermes/task3-b3-fact-reconciliation`, база `b11c277`, изменения не закоммичены.
|
||
Исходное ТЗ: `smm-task-B3-spec-2026-08-03.md` — остаётся в силе целиком, этот документ его не заменяет, а добавляет условия приёмки.
|
||
|
||
Пакет `Task3-B3-2026-08-04` — промежуточный (`STATUS-WIP.md` + первичный review), кода в нём нет. Проверку проводил по рабочей копии напрямую.
|
||
|
||
---
|
||
|
||
## 1. Что подтверждено независимо
|
||
|
||
Обе блокирующие находки первичного review — **реальны и в текущем состоянии рабочей копии не закрыты**. Проверял по коду, а не по отчёту.
|
||
|
||
### B1 подтверждён
|
||
|
||
`src/ServerMonitorManager.Control/LinkService.cs`:
|
||
|
||
```csharp
|
||
:416 if (persisted)
|
||
:418 await VerifyFactualStateAsync(current, expectedConnected, cancellationToken);
|
||
|
||
:431 else if (!persisted)
|
||
:433 current = current with { ActualState = completedState, LastError = null };
|
||
```
|
||
|
||
DB-less orphan удаляется сырым `link-disconnect`, после чего успех **присваивается синтетическому объекту без единой проверки факта**. Доказательства, что все handles с этим comment исчезли, нет.
|
||
|
||
Для persisted-политики проверка есть, но идёт через `link-status`, а `status_rule` по-прежнему вызывает `lookup_node_ip` для обоих узлов. Если Node отсутствует или не активирован, helper возвращает 80 **уже после успешного удаления правила**, и `ConvergeAsync:444-455` записывает `PendingActivation` / `mesh.node-not-activated`.
|
||
|
||
Дальше в `ReconcileAllAsync:274-284` классификация такова, что `PendingActivation` не попадает ни в `Converged`, ни в `Failed`. Возможен результат `Examined=1, Converged=0, Failed=0`, при котором фоновый сервис видит `Failed == 0` и **потребляет marker**, хотя фактический результат не подтверждён.
|
||
|
||
### B2 подтверждён
|
||
|
||
`deploy/ochenstarik-smm-policy-apply:37-44`:
|
||
|
||
```bash
|
||
lookup_node_ip() {
|
||
record="$(awk -F '\t' -v node="$node_id" '$1 == node { print; exit }' "$STATE_FILE")"
|
||
[[ -n "$record" ]] || node_not_activated
|
||
ip="$(awk -F '\t' '{ print $2 }' <<<"$record")"
|
||
[[ "$ip" =~ $ipv4_pattern ]] || fail "node has no valid mesh address: $node_id"
|
||
```
|
||
|
||
Читается только второе поле. Четвёртое — статус — не читается никогда. `reserve_node_address` пишет строку `node<TAB>10.77.0.x<TAB>-<TAB>reserved`, `peer-add` меняет статус на `active`. Значит зарезервированный, но не активированный Node проходит проверку, и `link-connect` создаёт accept-правило. Exit 80 возникает только при полном отсутствии строки — случай, которого в реальной топологии не бывает, потому что строку создаёт сам `node-code`.
|
||
|
||
Новый тест моделирует именно отсутствующую строку и поэтому дефект не ловит. Воспроизведение ревьюера на production-shaped фикстуре считаю корректным.
|
||
|
||
### Что уже сделано правильно и ломать не надо
|
||
|
||
- `disconnect_rule` больше не вызывает `lookup_node_ip` — удаление правила перестало зависеть от `nodes.tsv`. Это верное направление, сохранить.
|
||
- `list_rules` разбирает только managed-comment, проверяет арность 5 полей, node-паттерны, протокол и диапазон порта, отбрасывает подделки в stderr, чужие правила не выводит.
|
||
- `reconcile-status` возвращает `complete` при отсутствующем каталоге mesh; добавлена явная проверка `-x /usr/sbin/nft`; режим `SMM_POLICY_LISTING_FILE` для контрактных тестов.
|
||
- `Examined` / `Converged` / `Failed` разведены, `FailedPolicyIds` пробрасывается.
|
||
|
||
---
|
||
|
||
## 2. Обязательные исправления
|
||
|
||
### R1 — верификация факта только через `link-list` (закрывает B1)
|
||
|
||
Корень дефекта: проход стал fact-first, а верификация осталась per-link через `link-status`. Отсюда и пропуск проверки для DB-less, и зависимость от `nodes.tsv`, и лишние привилегированные вызовы.
|
||
|
||
Сделать:
|
||
|
||
1. `VerifyFactualStateAsync` переписать на `link-list`: считать число записей с ключом `(source, target, protocol, port)`.
|
||
- ожидается `Active` → **ровно 1**;
|
||
- ожидается `Disabled` или DB-less orphan → **ровно 0**;
|
||
- любое иное число — отказ применения с явным кодом.
|
||
2. Убрать условие `if (persisted)` вокруг верификации. DB-less orphan проверяется теми же правилами, что и persisted.
|
||
3. Убрать присваивание `current with { ActualState = completedState }` без предшествующей проверки. Синтетический объект имеет право получить `Disabled` только после подтверждённого нуля записей.
|
||
4. Верификация удаления **не должна** зависеть от наличия узла в `nodes.tsv`. `link-list` этого не требует — после перехода зависимость исчезает сама.
|
||
5. Оптимизация, которую даёт этот же переход: в проходе выполнять **один** финальный `link-list` после всех мутаций и сверять по нему все затронутые ключи разом, вместо вызова на каждую мутацию. Требование «проход без расхождений — один привилегированный вызов» из B3-2 сохраняется; для прохода с `k` мутациями допускается ровно два вызова (стартовый и верифицирующий).
|
||
|
||
### R2 — исчерпывающая классификация результата (закрывает вторую половину B1)
|
||
|
||
Ввести явный третий класс — `Deferred` — и инвариант:
|
||
|
||
```
|
||
Converged + Failed + Deferred == Examined
|
||
```
|
||
|
||
Инвариант проверять в коде (нарушение — исключение либо `LogError` с идентификаторами) и отдельным тестом. Ни одно состояние не должно молча выпадать из классификации.
|
||
|
||
**Решение, которое нужно принять явно, потому что иначе оно будет принято неправильно:**
|
||
|
||
- `PendingActivation` — это `Deferred`, а не `Failed`. Это ожидаемое устойчивое состояние: Node зарезервирован, `peer-add` ещё не выполнен, и так может продолжаться сутками.
|
||
- `Deferred` **не блокирует** потребление marker. Иначе один зарезервированный Node навсегда удержит маркер и вернёт ровно тот бесконечный цикл проходов каждые 30 секунд, который был закрыт в B-2 и B3-3.
|
||
- Marker блокирует только `Failed`, и на него уже действует ограничение в три попытки.
|
||
- `Deferred` обязан быть видимым: отдельное событие (`link.pending-node-activation` уже есть), отдельная подпись в Desktop, отражение в журнале прохода. Ошибкой не называется.
|
||
|
||
`LinkFullReconciliationResult` и `LinkReconciliationResult` дополнить полем `Deferred` и списком `DeferredPolicyIds` по образцу `FailedPolicyIds`.
|
||
|
||
### R3 — читать статус узла в `lookup_node_ip` (закрывает B2)
|
||
|
||
Разбирать минимум два поля — адрес и статус — и различать три исхода:
|
||
|
||
| Строка в `nodes.tsv` | Результат |
|
||
|---|---|
|
||
| статус `active`, валидный IPv4 | успех |
|
||
| статус `reserved` (или любой не-`active`) | exit **80**, точный маркер `mesh.node-not-activated` |
|
||
| статус `active`, IP пустой или невалидный | exit **78** — повреждённое состояние, fail-closed |
|
||
| строки нет вовсе | exit **80** |
|
||
|
||
Неизвестное значение статуса трактовать как не-`active` только если оно проходит валидацию формата; мусор в поле — это exit 78, а не 80. Fail-closed при сомнении.
|
||
|
||
Тесты обязаны использовать **production-shape** фикстуру из четырёх полей, разделённых табуляцией:
|
||
|
||
```
|
||
source 10.77.0.2 key-source active
|
||
target 10.77.0.3 - reserved
|
||
```
|
||
|
||
Три случая: `reserved` + валидный IP → 80; `active` + валидный IP → успех; `active` + невалидный IP → 78. Тест на полностью отсутствующую строку сохранить, но он не заменяет перечисленные.
|
||
|
||
### R4 — source-generated сериализация orphan audit (H1)
|
||
|
||
Заменить анонимный тип в `ControlStore.cs:1376-1398` именованным DTO, добавить его в source-generated JSON-контекст Control и сериализовать через `Context.Default.<Type>`. Мотив не в предупреждениях компилятора, а в том, что отказ произойдёт **после** мутации firewall и **после** публикации `link.orphan-removed`, дав противоречивую телеметрию.
|
||
|
||
Дополнительно к тому, что предложил review: проверить не локально, а прогоном опубликованного `linux-x64 PublishTrimmed` артефакта по orphan-пути и сверкой содержимого audit-записи. Разово, руками, с приложением вывода в `TEST_EVIDENCE.md`.
|
||
|
||
Порядок операций зафиксировать явно: запись audit — до публикации финального события успеха.
|
||
|
||
### R5 — Linux integration test на новый протокол (H2)
|
||
|
||
`tests/ServerMonitorManager.Control.Tests/LinkPolicyApplierIntegrationTests.cs` остался на протоколе `link-status`-first: fake helper не реализует `link-list`, ожидаемый журнал вызовов начинается с `link-status`. На Windows тест выходит по `OperatingSystem.IsLinux() == false`, поэтому заявленные локальные 85/85 дефект не показывают, а `linux-control-agent.yml` гоняет Control-тесты на Ubuntu, где пропуска не будет.
|
||
|
||
Сделать:
|
||
|
||
- реализовать `link-list` в fake helper;
|
||
- переписать ожидаемый журнал на fact-first протокол;
|
||
- проверить: no-drift → ровно один `link-list`; отсутствующая таблица классифицируется прямо на `link-list` без промежуточной мутации; мутация сопровождается финальной верификацией через `link-list`; DB-less raw disconnect проходит верификацию;
|
||
- прогнать Control suite **на Linux**, а не только на Windows.
|
||
|
||
Пока это не сделано, утверждение «CI зелёный» не имеет оснований: PR не создан, Linux-прогон не выполнялся.
|
||
|
||
### R6 — убрать fail-open default в интерфейсе (M1)
|
||
|
||
`ILinkPolicyApplier.ListRulesAsync` с телом по умолчанию, возвращающим пустой список, означает, что забытая реализация молча сообщает «firewall пуст» — для источника истины о факте это худший из возможных дефолтов, и он уже позволил старым test doubles скомпилироваться без нового протокола. Сделать `ListRulesAsync` и raw `ApplyDisconnectAsync(LinkRule, …)` обязательными членами интерфейса, все test doubles обновить явно.
|
||
|
||
### R7 — читаемость вложенных scope (L1)
|
||
|
||
Переформатировать `LinkService.cs:181-229` и `240-293` либо вынести обработку одного tuple и одного orphan в приватные методы без дублирования логики сходимости. Порядок блокировок `sorted node locks → per-Link gate` должен быть виден глазами, а не восстанавливаться разбором отступов.
|
||
|
||
---
|
||
|
||
## 3. Дополнительные тесты к обязательным из исходного ТЗ
|
||
|
||
1. DB-less orphan: helper сообщает успех удаления, но правило осталось → проход обязан классифицировать это как `Failed`, а не как успех.
|
||
2. Persisted `Disabled`, узел отсутствует в `nodes.tsv` → удаление подтверждается через `link-list`, состояние становится `Disabled`, а не `PendingActivation`.
|
||
3. Marker не потребляется, если хоть один элемент попал в `Failed`; потребляется, если остались только `Converged` и `Deferred`.
|
||
4. Инвариант `Converged + Failed + Deferred == Examined` на смешанном наборе из шести политик.
|
||
5. Проход с `k` мутациями выполняет ровно два привилегированных вызова `link-list`.
|
||
6. Production-shape `reserved` → exit 80 (см. R3).
|
||
7. Зарезервированный Node в течение десяти последовательных проходов: `Deferred` каждый раз, marker потребляется, число проходов не превышает расписание.
|
||
|
||
---
|
||
|
||
## 4. Definition of Done B-3R
|
||
|
||
- [ ] Верификация факта выполняется только через `link-list`, счётом записей, одинаково для persisted и DB-less
|
||
- [ ] Синтетический успех без проверки факта отсутствует в коде
|
||
- [ ] Верификация удаления не зависит от `nodes.tsv`
|
||
- [ ] `Converged + Failed + Deferred == Examined` выполняется и проверяется тестом
|
||
- [ ] `PendingActivation` классифицирован как `Deferred`, marker им не блокируется, состояние видно в UI и журнале
|
||
- [ ] `lookup_node_ip` читает статус; `reserved` → 80, `active` + битый IP → 78; тесты на production-shape фикстуре
|
||
- [ ] orphan audit сериализуется через source-generated контекст; проверено на опубликованном trimmed артефакте
|
||
- [ ] Linux integration test переведён на fact-first протокол и **прогнан на Linux**
|
||
- [ ] `ListRulesAsync` и raw disconnect — обязательные члены интерфейса
|
||
- [ ] Все пункты DoD исходного ТЗ B-3 по-прежнему выполняются
|
||
- [ ] PR создан, CI зелёный — впервые для этой ветки
|
||
- [ ] Финальный независимый review получает **полный текст исходного ТЗ B-3 и этот документ**, а не только диф
|
||
|
||
---
|
||
|
||
## 5. Формат сдачи
|
||
|
||
Как в предыдущих блоках: `REPORT.md`, `TEST_EVIDENCE.md`, патч и/или ZIP, `CI_CHECKS.txt`, финальный `INDEPENDENT_REVIEW.md`, `SHA256SUMS` — считать последним действием, после всех правок отчётов, включая все файлы пакета.
|
||
|
||
Промежуточный пакет из двух файлов, как сейчас, — нормальная практика для отчёта о ходе работ, если он так и назван. Финальный обязан быть полным.
|
||
|
||
Статус по итогам merge формулировать как `merged / verified, physical acceptance pending`.
|
||
|
||
---
|
||
|
||
## 6. Оценка первичного review
|
||
|
||
Обе блокирующие находки верны, воспроизведение B2 корректно, разбор границ безопасности содержательный. Ревьюер честно указал, что `dotnet` в его окружении отсутствует и заявленные 85/85 он не воспроизводил — и именно из этого сделал правильный вывод про H2, что Linux-прогон обязателен. Это ровно тот способ работы, который нужен.
|
||
|
||
Практика передачи ревьюеру полного текста ТЗ продолжает окупаться: B2 и H2 — пропущенные требования, а не ошибки в написанном коде, при diff-only ревью они не находятся.
|
||
|
||
---
|
||
|
||
## 7. Внешний блокер, без изменений
|
||
|
||
Physical acceptance не выполнялся ни разу за четыре блока. Нужны `HUB_SSH_HOST`, `HUB_SSH_USER`, `SOURCE_SSH_HOST`, `SOURCE_SSH_USER`, `HOME_WG_IP`, `SECOND_WG_IP`, `SSH_IDENTITY_FILE` и прогон
|
||
|
||
```bash
|
||
SMM_ACCEPT_RESTORE=1 SMM_ACCEPT_REBOOT=1 tests/acceptance/three-server-mesh.sh
|
||
```
|
||
|
||
Harness готов и содержит и firewall-restore, и инъекцию orphan-правила. Критерий B6 и пункт «выполнить физический acceptance» Этапа 3 roadmap до этого прогона остаются открытыми при любом состоянии CI. Исполнитель закрыть их не может.
|
||
|
||
Следующий блок после B-3R — **Блок C**, доверенная поставка, по разделу 3 документа `smm-task-4-2026-08-03.md`.
|