av
8f7c3a057a
- убраны клиент бота, транспорт обновлений, отправитель сообщений, сборка входа при старте, секция настроек и зависимость go-telegram-bot-api; из конвейера ушла доставка ответа отправителю — исход виден опросом готовности. Колонки адресата и значение источника остались в схеме: применённые шаги не переписываются - шаг 202608140003 запрещает пустого владельца у аудиозаписи и у файла; существующие строки он не проверяет, и это принято сознательно — искать их надо запросом до выкладки - ревью нашло два пред-существующих дефекта, оба закрыты: пустой второй ответ распознавателя стирал сохранённую расшифровку, а пустая расшифровка перестала быть заметной вместе с убранной доставкой. Попутно поднят golang.org/x/image до v0.45.0 — красный шаг vulns, воспроизводился и на чистом master
193 lines
18 KiB
Markdown
193 lines
18 KiB
Markdown
# Ревью remove-telegram-intake — триаж
|
||
|
||
## Сводка
|
||
|
||
- **Режим:** по графу. **Метка:** `large` (разметка `review-scope`). 50 файлов, −2394 строки, семь
|
||
удалённых файлов кода, новый шаг схемы; необратимый элемент — шаг схемы, переписаны инварианты
|
||
`CLAUDE.md`.
|
||
- **База диффа:** `origin/master` отстала на два закрытых изменения; сверка шла против `HEAD` плюс
|
||
незакоммиченное рабочее дерево.
|
||
- **Гейт:** зелёный целиком (`task gate` → 0).
|
||
- **Сигнал о заниженной метке:** `review-code` возражений не подал. `review-basics` не запускался —
|
||
второго независимого голоса о метке нет.
|
||
- **На вход:** 22 находки (specs 5, code 8, architecture 3, ops 2, adversary 4, autotests 0) плюс
|
||
4 замечания. После дедупликации по причине — 11 различных причин, одна выброшена как ошибочная.
|
||
|
||
### План с исходом по каждой теме
|
||
|
||
| тема | дом | глубина | кто закрывает | исход |
|
||
| --- | --- | --- | --- | --- |
|
||
| requirements | `openspec/specs/{intake,pipeline,access,storage}` + дельты | разбор | specs | закрыта, 5 находок |
|
||
| autotests | `CLAUDE.md` «Гейт»/«Команды» | — | autotests | закрыта, 0 находок + замечание о дрейфе `go-linters.md` |
|
||
| conventions + техника | `docs/conventions/` | разбор | code | закрыта, 7 находок + 1 ошибочная |
|
||
| architecture | `docs/architecture.md` (+ `passport.md`) | доказательство | architecture | закрыта, 3 находки |
|
||
| security | `docs/security.md` | доказательство | adversary | закрыта, 4 находки + 3 свойства без пути |
|
||
| operations | `docs/architecture.md` «Эксплуатация» (+ `database.md`) | доказательство | ops | закрыта, 2 находки |
|
||
|
||
Тем без отчёта нет. Тем без дома нет. `basics` не запускался — своих тем сверх ядра план ему не дал.
|
||
|
||
## Блокирует мердж
|
||
|
||
### Ничья запись переживёт новый шаг схемы и станет незакрываемой
|
||
|
||
- Файл: `internal/adapter/repo/pocketbase/migrations/202608140003_owner_required.go:20-30`;
|
||
`openspec/changes/remove-telegram-intake/design.md`, «Migration Plan», п. 1
|
||
- Severity: major. Confidence: high
|
||
- Оракул (переснят триажем, `go test ./internal/adapter/repo/pocketbase/ -run TestTriageProbeOwnerRequiredOverOwnerlessRow`):
|
||
|
||
```
|
||
PRE-STEP: ничья запись заведена, owner=""
|
||
STEP 003 (Required=true) поверх ничьей записи: err=<nil>
|
||
ПОСЛЕ ШАГА: строка на месте, owner=""
|
||
FindAndAcquire: выдана запись
|
||
Save остановленной ничьей записи: err=failed to update audio record: owner: cannot be blank.
|
||
```
|
||
|
||
- Последствие: `Required` у поля связи PocketBase — проверка при сохранении записи, а не ограничение
|
||
таблицы. Шаг проходит зелёным на базе с ничьей записью, и запись остаётся. Дальше она выдаётся
|
||
воркеру (захват идёт сырым запросом мимо валидации), любое сохранение падает, `halt()` перехватывает
|
||
отказ до строк, растящих `WorkerJobCounter` и пишущих `record_events`. Запись не останавливается,
|
||
берётся снова по истечении срока захвата и повторяется неограниченно: ни метрики, ни журнала
|
||
событий, только строка в логе контейнера. Предвыкладочная проверка, на которой стоит безопасность
|
||
шага, не отличает чистую базу от грязной.
|
||
- Найдено проходами: specs, code, ops (три прохода, одна причина).
|
||
- **Действие: развилка.** Три варианта: (1) шаг сам считает ничьи строки и отказывается;
|
||
(2) шаг остаётся, утверждение «падает на живой базе» уходит из комментария и плана, а в порядок
|
||
выкладки добавляется ручная проверка запросом; (3) шаг приводит данные — необратимо, решение
|
||
владельца обязательно.
|
||
|
||
### Второй ответ провайдера, приехавший пустым, стирает сохранённую расшифровку
|
||
|
||
- Файл: `internal/service/transcribe.go:716-733` (`poll` → `storeOutcome`),
|
||
`internal/adapter/repo/pocketbase/text_repo.go:40-53`
|
||
- Severity: critical. Confidence: high
|
||
- Оракул: падающий тест, переснят триажем — `expected: "Личный разговор." actual: ""`.
|
||
- Последствие: поток gRPC SpeechKit, закрывшийся на первом `Recv`, отказом не считается — `outcome`
|
||
пуст, `err == nil`. `Texts.Put` кладёт пустое поверх сохранённого безусловно, рубеж двигается,
|
||
запись доходит до `done` без текста. Сырой ответ уцелеет (`Finish` пишет вложение под условием
|
||
`len(raw) > 0`), но `ReadRaw`/`Parse` в боевом коде не зовёт никто.
|
||
- **Регрессией этого изменения не является:** `poll`, `storeOutcome` и `text_repo.go` диффом не
|
||
тронуты. Изменение сняло последний видимый признак вырожденного ответа — заглушку «на записи нет
|
||
текста», — но она уходила только в Telegram.
|
||
- Найдено проходом: adversary.
|
||
- **Действие: развилка.** (1) `storeOutcome` не пишет пустое поверх непустого; (2) вырожденный ответ
|
||
считается отказом шага; (3) задача, мердж как есть.
|
||
|
||
## Стоит исправить сейчас
|
||
|
||
### Пустая расшифровка перестала замечаться, а два документа обещают, что она замечена
|
||
|
||
- Файл: `internal/service/transcribe.go` (`finish`), `docs/conventions/logging.md:62`,
|
||
`docs/architecture.md:142`
|
||
- Severity: major. Confidence: high
|
||
- Оракул: `logging.md:62` называет «пустой текст распознавания» поимённым примером уровня `WARN`;
|
||
`architecture.md:142` утверждал «запись завершается заглушкой „на записи нет текста"». Заглушку
|
||
изменение убрало вместе с доставкой, замены не было.
|
||
- **Действие: инлайн. Исправлено:** `poll` пишет `WARN` с идентификатором записи при пустом
|
||
результате; строка `architecture.md:142` переписана на фактическое поведение; заведён оракул
|
||
`TestEmptyRecognitionIsNamedInJournal`.
|
||
|
||
### Приёмка переписанного инварианта «остановленная запись несёт причину» не может упасть
|
||
|
||
- Файл: `internal/service/pipeline_test.go`
|
||
- Severity: minor. Confidence: high
|
||
- Оракул: `Halt()` ставит `HaltedAt` и `HaltReason` одним движением, `IsHalted()` читает `HaltedAt` —
|
||
значит `require.NotNil(HaltReason)` следует из `require.True(IsHalted())`. Независимый сигнал
|
||
(`sender.sent()`) убран вместе с доставкой.
|
||
- **Действие: инлайн. Исправлено:** вторым утверждением взята строка журнала событий — отдельное
|
||
сохранение, способное упасть само по себе; добавлена третья причина остановки.
|
||
|
||
### Удаление входа не доведено: подавление линтера, конвенции, фикстуры и комментарии
|
||
|
||
- Файл: `.golangci.yml:158`; `docs/conventions/go-linters.md:81,92,155`; `internal/contract/error.go`;
|
||
`internal/config/config_test.go`; `internal/service/transcribe.go`; `internal/contract/repository.go`;
|
||
`internal/metrics/format_label.go`; `internal/service/ownership_test.go`;
|
||
`openspec/specs/pipeline/spec.md:104`
|
||
- Severity: minor. Confidence: high
|
||
- Последствие: подавление `errcheck` по мёртвому символу — заряженная мина: вход убран временно, метод
|
||
`send` вернётся под тем же именем и молча окажется без проверки отказов. Остальное — документы и
|
||
комментарии, обещающие ответ отправителю, включая нормативный доклад `contract/repository.go`.
|
||
- Найдено проходами: specs, code, architecture, autotests (одна причина, четыре прохода).
|
||
- **Действие: инлайн. Исправлено целиком:** снято подавление и строки про `send`; `controller/tg`
|
||
убран из таблицы архправил; удалён `ErrDeliveryChannelDown`; переписаны комментарии; фикстуры
|
||
переведены с мёртвой секции; требование «Захват задачи неделим» внесено в дельту.
|
||
|
||
### Анонимный запрос кладёт до мегабайта своего текста в журнал контейнера одной строкой
|
||
|
||
- Файл: `main.go:184-202`
|
||
- Severity: major. Confidence: high
|
||
- Оракул: живой прогон — 900000 байт в пути → 404, журнал вырос с 1048 до 901194 байт.
|
||
- Последствие: длину строки журнала задаёт неузнанный посетитель; разбор инцидента по журналу
|
||
становится невозможен. Тот же класс назван недопустимым в `internal/controller/http/auth.go:277-284`.
|
||
- **Дефект пред-существующий:** `main.go` этим изменением правится только в части подъёма входов.
|
||
- **Действие: развилка.** Чинить здесь или заводить задачей.
|
||
|
||
## Гипотезы без доказательства
|
||
|
||
- **`transcribed` — лишний рубеж** (architecture): готовая расшифровка платит отдельный захват и
|
||
попадает под сторож застревания за проход, переставляющий одну колонку. Оракула, что это случалось,
|
||
нет. Вопрос владельцу — место ему в задаче.
|
||
- **Отказ приёма по пустому владельцу приходит новому вызывающему как 500** (specs): путь сегодня
|
||
недостижим, транспорт отвергает раньше.
|
||
- **`http.status_code=0` у всякого отвергнутого запроса** (adversary, `main.go:194-199`).
|
||
Пред-существующее.
|
||
- **Хвост имени отправителя уезжает в журнал внутри поля `error`** (adversary): инвариант приватности
|
||
не нарушен, нарушена форма изъятия. Пред-существующее.
|
||
- **Три свойства без построенного пути** (adversary): длительность из метаданных ничем не ограничена
|
||
(переполнение `int`); непустой, но более короткий ответ провайдера затирает и вложение;
|
||
`POST /api/collections/users/request-verification` открыт анониму.
|
||
|
||
**Выброшено как ошибочное:** находка `code` о `gofmt` на `pipeline_test.go` — `gofmt -l .` пуст,
|
||
проверено дважды.
|
||
|
||
**Выброшено как вкусовщина:** переименование `CreateJobFromApi`.
|
||
|
||
**Проектные ложноположительные:** строка «Запись без владельца не достаётся никому» в `docs/review.md`
|
||
отменена дважды, последний раз этой же задачей — находка о ничьей записи ей подтверждается, а не
|
||
отсеивается.
|
||
|
||
## Promote candidates
|
||
|
||
- **В `docs/conventions/logging.md`:** длину поля журнала не задаёт вызывающий — всё, что приходит
|
||
извне, уезжает в журнал усечённым до объявленного предела. Правила нет, случай второй.
|
||
- **Отклонённый кандидат:** страж существования символов в `exclude-functions` — заводить нельзя,
|
||
`CLAUDE.md` «Проверок над проверками не заводить» запрещает стражей предмета у правил.
|
||
|
||
## Границы покрытия
|
||
|
||
**Что запускалось.** Шесть проходов на метке `large`, режим по графу: specs, autotests, code,
|
||
architecture, adversary, ops. `basics` не запускался — план не дал ему тем сверх ядра. Корректор метки
|
||
(`review-code`) отработал, возражений не подал; второго корректора в прогоне не было.
|
||
|
||
**Потолки проходов.** Ни один проход не сообщил свой потолок и что осталось за срезом, хотя контракт
|
||
обязывает. Неизвестно, показал ли `code` все находки или только верхние.
|
||
|
||
**Что не влезло в потолок триажа** (названо, а не выброшено):
|
||
|
||
- откат образа после вычистки секции `[telegram]` из боевого конфига роняет старт прежнего бинаря
|
||
(`ops`, оракул — прогон исторического бинаря `9a964f2`);
|
||
- ряд метрики `intake_up{telegram}` исчезает, а не обнуляется — цена не названа в документах владельца;
|
||
- `down202608140003` не покрыт (0.0%) — так у всех четырёх шагов `down*`, и они операционно
|
||
недостижимы: cobra-команды PocketBase не подключены;
|
||
- частичное покрытие двух требований дельты: «Поднятые входы видны наблюдателю» (оракул только живой)
|
||
и «Приведённая копия получает владельца записи» (косвенно).
|
||
|
||
**Что осталось целиком на человеке** — из `docs/review.md`, «Недоступно проверке»: поведение внешних
|
||
сервисов под нагрузкой и на границах, реальный профиль нагрузки, стойкость `ffmpeg` к вредоносному
|
||
входу, поведение настоящей Authelia, поведение браузера с куками. Сознательно перестали проверять:
|
||
разбор вывода настоящего `ffprobe`; работа с настоящими SpeechKit и Object Storage; вход через живого
|
||
провайдера OIDC.
|
||
|
||
**Четыре строки, которые в конвейере не закрывает никто:**
|
||
|
||
1. Решения проекта не сверялись — `docs/adr/` процессный, расхождение ловит `av-dev:doc-healthcheck`.
|
||
2. Записанные наблюдения (`docs/research/`) не использовались; всякое число снято на этом прогоне.
|
||
3. Поимённая сверка с руководствами по стилю Go не задавалась ни одним проходом.
|
||
4. Альтернативной реализации, с которой можно сдиффить решения, у конвейера нет.
|
||
|
||
**Чем работал триаж.** Две пробы на копии дерева в скретчпаде, базы — временные каталоги. Боевых
|
||
данных, боевых ключей и выкладки не касался.
|
||
|
||
Формулировки «критичных проблем не обнаружено» в отчёте нет: одна `critical` подтверждена падающим
|
||
тестом и живёт в сервисе прямо сейчас.
|