Files
transcriber/openspec/changes/archive/2026-08-11-fix-http-handler-tests/review/triage.md
T
av 6c04c801c9 http: тесты приёма переписаны на подставные адаптеры
- проверки больше не зовут ffprobe и не меняют рабочий каталог процесса;
  добавлены случаи на отказ чтения метаданных и на отсутствие поля audio
- заведена спека intake на приём по HTTP, ADR о подставных адаптерах,
  запись в журнал ревью о проверке, которая не могла упасть
- go test снят из объявленных долгов CLAUDE.md, послабление errcheck
  для _test.go в .golangci.yml убрано
2026-08-11 08:35:11 +03:00

34 KiB
Raw Blame History

Триаж ревью: fix-http-handler-tests

Сводка

  • Размер: малое. Сложность: знакомое. Метка: small. Обоснование разметки (review-scope): изменение трогает один тестовый файл и одну строку конфига линтера, поведение сервиса не меняется, отрицательный тест метки (миграция, формат файла на диске, публичный контракт API, имя ключа) не срабатывает — контракт HTTP API спекой фиксируется, а не меняется.
  • Режим прогона: по графу. Дизайн — одна стадия (specs), код — autotestsspecs + code → триаж.
  • Сигнал о заниженной метке: пришёл от review-code — возражений нет, метку small проход счёл обоснованной. review-basics не запускался (своих тем у проекта нет), второго независимого подтверждения метки нет.
  • Состояние гейта (проверено мной на этом прогоне):
    • go test ./...зелёный (ok internal/controller/http 0.013s); это и есть цель изменения;
    • golangci-lint run4 замечания, ровно объявленный долг (speechkit.go:55, main.go:124, worker.go:51, service/transcribe.go:394). Ни одного нового, в том числе после снятия исключения errcheck для _test.go;
    • критерий приёмки «не зависит от ffprobe» подтверждён: собранный go test -c бинарь проходит под env -i PATH=<пустой каталог>;
    • критерий «os.Chdir не остаётся» подтверждён: grep -rn 'os.Chdir' internal/ пуст.
    • Итог: гейт красный только унаследованным долгом; новых красных шагов нет.

План разметки с исходом по каждой теме

тема дом глубина закрывает исход
requirements openspec/changes/fix-http-handler-tests/specs/intake/spec.md сверка specs закрыта, 3 находки (потолок 3/3 сработал), 2 из них починены и мной перепроверены мутацией
autotests CLAUDE.md § Гейт autotests закрыта, 0 находок; отчёт о гейте + 1 строка в границы покрытия. О своём потолке проход не сообщил
conventions docs/conventions/README.md (на small — только README) сверка code закрыта, 1 находка (потолок 1/2), не починена — единственный блокер ниже
architecture CLAUDE.md § Инварианты сверка code закрыта, 0 находок (потолок инвариантов 0/1)
security CLAUDE.md § Инварианты сверка code закрыта, 0 находок — но см. предупреждение ниже
operations CLAUDE.md § Инварианты сверка code закрыта, 0 находок

Тем без отчёта нет. Тем без дома нет.

Предупреждение по теме security. «0 находок» здесь не значит «чисто». Нарушение critical-инварианта «содержимое записи остаётся приватным» (internal/service/transcribe.go:107 пишет "file_name", fileName — имя файла пользователя — в журнал) найдено на стадии дизайна и сознательно отложено решением человека на чекпоинте, поэтому проход кода вернул пустой итог: находка уже известна и вынесена. Она в урожае, не в блокерах, и это решение человека, а не моё. Тот же путь проходят записи из Telegram.

Арифметика

  • На входе: 14 позиций — 7 именованных находок (specs 3, code 4), 3 названные ниже потолка, 3 отложенные решением человека, 1 замечание о непокрытом конвейере от autotests.
  • После дедупликации, добычи оракулов и отсева: 2 позиции, требующие действия (1 блокер + 1 развилка). Остальное — в урожай, гипотезы и promote, ничего не выброшено молча.
  • Починенное проверено мной независимо: обе major-находки specs закрыты, мутации их роняют (оракулы ниже).

Блокирует мердж

CLAUDE.md продолжает объявлять долгом отказ, которого больше нет, — и следующий настоящий отказ тестов приёма спишут на него молча

  • Файл: CLAUDE.md:96-101; сопутствующее: tasks/BACKLOG.md:23, tasks/items/http-handler-tests-never-green.md
  • Severity: major
  • Confidence: high
  • Действие: инлайн
  • Оракул (мой, на этом прогоне):
    • дословно CLAUDE.md:98-101: «go test ./... падает в internal/controller/http: тесты требуют testdata/sample.m4a, которого в репозитории нет и не было… Заведено задачей http-handler-tests-never-green»;
    • go test ./...ok git.vakhrushev.me/av/transcriber/internal/controller/http 0.013s;
    • дословно CLAUDE.md § Работа: «Два объявленных долга из раздела „Гейт“ сломанным состоянием не считаются, пока их не закрыли задачами».
  • Последствие: после мерджа проект будет письменно утверждать, что красный go test в internal/controller/http — это норма. Ровно этот механизм записан в журнале дефектов (docs/review.md, запись 2026-08-10): «тест, который никогда не проходил, обнуляет сигнал всего пакета: настоящий отказ в нём становится неотличим от привычного шума». Изменение восстанавливает сигнал в коде и оставляет его выключенным в документе, по которому судят «сломано ли». Отказ будет молчаливым: никто не станет разбираться в отказе, объявленном известным.
  • Предложение: убрать первый из двух известных отказов в CLAUDE.md § Гейт (остаётся только golangci-lint), закрыть задачу штатным путём каталога (tasks.py close — реализованные в REJECTED.md не идут, у них есть коммит), снять строку из tasks/BACKLOG.md. Задача tasks.md этого шага не содержит — добавить его в чек-лист.
  • Найдено проходом: review-code/конвенции; оракул и провенанс — триаж.
  • Почему блокер, а не «стоит исправить»: CLAUDE.md § Работа — единственное место, где записано, что считается сломанным. Пока оно врёт, определение сделанного у следующей задачи опирается на неверный список. Правка механическая, путь документирован, цена — минуты.

Замечание о разделении обязанностей. Общая согласованность документов между собой и с кодом — работа скилла av-dev-docs:healthcheck, а не ревью (CLAUDE.md § Гейт говорит это прямо). Здесь исключение узкое и обосновано: речь не о дрейфе документации вообще, а о том, что это самое изменение закрывает долг, поимённо перечисленный в CLAUDE.md, и без правки определение «сломано» становится ложным в момент мерджа.


Стоит исправить сейчас

Переименование маршрута POST /api/audio в main.go уедет зелёным: тест ходит по своей копии регистрации

  • Файл: main.go:198-202 против internal/controller/http/transcribe_test.go:140-144
  • Severity: minor
  • Confidence: high
  • Действие: развилка
  • Оракул (мой, мутация в копии дерева, /tmp/.../scratchpad/mut): api.POST("/audio", …)api.POST("/upload", …) в main.go; go build ./... проходит, go test ./internal/controller/http/ -count=1ok … 0.017s. Тест зелёный при сломанном контракте. Для сравнения — то, что теперь ловится: переименование тега json:"job_id"json:"jobId" роняет TestCreateTranscribeJob_Success («does not contain "job_id"»), удаление s.jobRepo.Create(job) роняет его же.
  • Последствие: имена полей ответа изменение защитило (это и была починенная major-находка), а путь маршрута — часть того же публичного контракта HTTP API, объявленного в CLAUDE.md необратимым, — остался незащищённым. Внешняя программа сломается молча, машина промолчит. Вероятность невысока (мутация видна в диффе main.go), но класс тот же самый.
  • Развилка для человека — правка трогает продуктовый код, а design.md объявил это Non-Goal («код internal/service и internal/controller/http не правится»):
    1. Вынести регистрацию маршрутов в экспортируемую функцию пакета internal/controller/http (например, RegisterRoutes(r gin.IRouter, h *TranscribeHandler)), звать её из main.go и из сборки теста. Цена: ~10 строк продуктового кода, выход за объявленный Non-Goal задачи, зато контракт маршрута закрыт машиной.
    2. Оставить как есть, записать в урожай отдельной задачей. Цена: контракт маршрута остаётся на человеке до следующей задачи, которая и так трогает main.go (например, json-api-for-spa).
    3. Оставить как есть и не заводить задачу, приняв, что маршрут проверяется глазами. Цена: класс дефекта известен, но не записан нигде — при следующем промахе оракула не будет.
  • Найдено проходом: review-specs (там — minor, «цена исправления выше цены дефекта»); оракул мутацией — триаж.

Второго пункта в этой секции нет: остальное либо починено и перепроверено, либо не имеет цены, оправдывающей правку (см. «Отсеяно» и «Урожай»).


Гипотезы без доказательства

Понижено оракулом: «зелёный прогон печатает ERROR-строки в stderr» — заявленного последствия нет

  • Исходно: review-code/техника, minor. Часть починена (логгер сборки теста уведён в io.Discard), остаток — log.Printf("Err: %v", err) в internal/controller/http/transcribe.go:45.
  • Мой оракул: go test ./internal/controller/http/ -count=1 2>&1 | grep -c 'Err:'0. go test буферизует вывод пакета и на успехе его не печатает. Строка видна только под -v или при прямом запуске собранного бинаря — то есть на зелёном task gate признак ничем не размывается.
  • Итог: последствие в формулировке находки не воспроизводится, находка снята. Остаётся факт «продуктовый код пишет мимо slog» — он уже записан в docs/conventions/logging.md:161 как расхождение, новой находкой не является, ушёл в promote (механизация правила).

Понижено: «база :memory: без ограничения пула»

  • Исходно: review-code/техника, minor, Confidence: low, «сегодня не срабатывает». Оракула, показывающего отказ, нет ни у прохода, ни у меня. Починка (db.SetMaxOpenConns(1), одна строка, с комментарием почему) уже внесена, безвредна и оставлена как есть. Действия не требует.

Не понижалось, но перепроверено: две major-находки specs

Обе были заявлены с оракулом и обе починены. Я не поверил на слово и повторил мутации в копии дерева — см. оракул в секции «Стоит исправить сейчас». Обе мутации теперь роняют тест. Находки закрыты.


Отсеяно

  • Мёртвая строка router.MaxMultipartMemory в сборке теста (transcribe_test.go:138, названо review-code ниже потолка). Проверено: гиновский MaxMultipartMemory читается только в c.FormFile/c.MultipartForm, а обработчик зовёт c.Request.FormFile, который использует собственный defaultMaxMemory = 32 MiB — то же число. Поведение не меняется ни в тесте, ни в main.go, стоимость следующего изменения не растёт, записанной конвенции нет. Выброшено, а не смягчено.
  • Проектных ложноположительных (docs/review.md → «Типовые ложноположительные», 4 пункта) в выводах не оказалось ни одного. Ближайший сосед — «Файлы и объекты не удаляются, диск растёт» — к отложенной находке про файл-сироту не относится: та запись про отсутствие срока хранения, а находка — про файл, на который нет ни задачи, ни записи в учёте. По этому пункту ничего не отсеяно.

Promote candidates

  1. Имена полей публичного ответа судятся по сырому JSON, а не по разобранной структуре. Приём в разобранную структуру переименовывает тег вместе с ожиданием, и проверка теряет способность упасть — это ровно та major, что нашлась здесь. Приём (map[string]json.RawMessage + assert.Contains) сработал дважды в одном файле. Дома у правила пока нет: в docs/conventions/ файла про тесты нет. Кандидат в новый раздел конвенций.
  2. Стандартный log в продуктовом коде — механизировать, а не помнить. docs/conventions/logging.md уже пишет «Механизировано: ничего. Ни sloglint, ни forbidigo в .golangci.yml не заведено» и поимённо называет расхождение internal/controller/http/transcribe.go. Правило записано и механизируемо, значит это не находка ревью, а Promote candidate: forbidigo на log. в .golangci.yml.
  3. Регистрация маршрутов — одна на процесс и на тест (производное от развилки выше; актуально, если человек выберет вариант 2 или 3).

Урожай

Формулировка → оракул → провенанс. Ничего из этого не чинится в этом изменении.

  1. Приём пишет имя файла пользователя в журнал — нарушение critical-инварианта. internal/service/transcribe.go:107: s.logger.Info("Creating transcribe job", "file_id", …, "file_name", fileName, …). Оракул: дословно CLAUDE.md § Инварианты — «Содержимое записи остаётся приватным. Текст расшифровки, имя файла пользователя и его сообщение в лог не пишутся — только длина и идентификаторы. Нарушение необратимо: строки уже уехали в журнал контейнера. critical». Плюс вопрос темы security в docs/review.md. Путь общий с Telegram, то есть касается живых записей. Провенанс: ревью дизайна; отложено решением человека на чекпоинте, записано в design.md § Risks. Спека intake нормирует только хранилище и о журнале молчит намеренно.
  2. Отказ чтения метаданных оставляет файл на диске без уборки. internal/service/transcribe.go:129-133 (ветка metaviewer.GetInfo) против 152-157 (ветка fileRepo.Create, где os.Remove есть). Оракул: чтение кода; ни задачи, ни записи в учёте под такой файл нет — сопоставить его не с чем. Провенанс: ревью дизайна; отложено решением человека (правка трогает три ветки отказа и меняет поведение на диске).
  3. Настоящий ffprobe после этой правки не проверяется ничем. У internal/adapter/metaviewer/ffmpeg своего теста нет; разбор вывода ffprobe не покрыт. Оракул: go test ./...? …/adapter/metaviewer/ffmpeg [no test files]. Провенанс: design.md § Risks, названо прямо. Формально покрытие не потеряно — прежний тест проверял отказ ffprobe и выдавал его за проверку приёма.
  4. Конвейер задач не покрыт ни одним тестом. FindAndRunConversionJob, FindAndRunTranscribeJob, FindAndRunTranscribeCheckJob, internal/controller/worker. Три воркера читают общий *sql.DB, теста с параллельным доступом нет. Оракул: go test ./...[no test files] у internal/service и internal/controller/worker. Провенанс: autotests, вне scope задачи. Совпадает со свойством, добавленным в docs/review.md («изменённое место покрыто хоть одним проходящим тестом») и с вопросом темы autotests.
  5. Опечатка transcibe в тексте ошибки теперь закреплена проверкой. internal/controller/http/transcribe.go:46 и transcribe_test.go:379"Failed to create transcibe job". Оракул: обе строки дословно. Исправление меняет тело ответа, то есть публичный контракт HTTP API, объявленный в CLAUDE.md необратимым, — значит спрашивается у человека и не делается походя. Провенанс: review-specs, ниже потолка. Действия сейчас не требует: статус-кво зафиксирован сознательно.
  6. Требование «имя отправителя не попадает в хранилище» проверяется только благополучными именами. Проверено мной попутно (вопрос темы security из docs/review.md: «не строится ли путь на диске из значения, пришедшего снаружи»): обхода каталога нет по построениюfilepath.Ext не пересекает разделитель пути, и на входах ../../../etc/passwd.m4a, evil.m4a/../../x, /etc/passwd, a.b/../../c, .. результат всегда <uuid><ext> внутри каталога хранения (оракул: прогон filepath.Ext + filepath.Join на этих восьми входах, вывод снят на этом прогоне). Дефекта нет; недостающее — сторожевой случай, который зафиксирует это свойство. Провенанс: триаж.

Границы покрытия

План: темы, дома, глубины

Полностью воспроизведён в сводке выше вместе с исходом каждой темы. Тем без дома нет, тем без отчёта нет. Дома тем architecture, security и operations — раздел «Инварианты» CLAUDE.md, глубина «сверка».

Какие проходы запускались

  • Ревью дизайна: review-specs, одна стадия, метка small.
  • Ревью кода: review-autotestsreview-specs + review-code → триаж. Режим — по графу, метка small.
  • Не запускался review-basics: своих тем у проекта нет — так сказал план. Следствие названо ниже, в строке про корректор метки.

Сработавшие потолки

  • review-specs (код): 3 из 3, потолок сработал. Ниже среза остались названными: литералы сообщений об ошибке, включая опечатку transcibe; расхождение сценария «Поля с записью нет» с тем, что делал тест (починено).
  • review-code: техника 3/3 — сработал; конвенции 1/2; инварианты 0/1. Ниже среза осталась названной мёртвая строка router.MaxMultipartMemory.
  • review-autotests: о своём потолке не сообщил. Это находка о прогоне: по контракту проход обязан сказать, сколько нашёл, каков был потолок и что осталось за срезом. Судить, есть ли за его срезом что-то ещё, нечем.
  • Триаж: потолок 3/4 не исчерпан (1 блокер, 1 в «стоит исправить»). Из-за потолка ничего не выброшено.

Что каждый запущенный проход не мог проверить в принципе

Ниже — по отчётам проходов; charter'ы агентов мне дословно не подавались, поэтому это пересказ их собственных заявлений, а не цитата устава.

  • review-autotests: судит наличие и зелёность проверок, а не правильность нормы, которую они проверяют. Прогнал go test 5× подряд и с -race — флаки не обнаружен; это отсутствие сигнала на пяти прогонах, а не доказательство детерминированности.
  • review-specs: судит соответствие кода дельта-спеке; правильность самой спеки вне его входа. Приём из Telegram спекой intake не описан сознательно — значит, и не проверялся.
  • review-code: на метке small конвенции сверялись только с docs/conventions/README.md, а architecture/security/operations — только с записанными инвариантами CLAUDE.md.
  • Триаж: ничего нового не находит по определению. Я не читаю код в поисках дефектов, я работаю с чужими выводами. Пропуск любого прохода — мой пропуск тоже; всё, что я могу, — назвать его поимённо, что и сделано выше.

Что осталось целиком на человеке

Из docs/review.md → «Недоступно проверке», двумя отдельными списками, как записано:

Не проверит ни один проход:

  • operations: поведение внешних сервисов под нагрузкой и на границах — SpeechKit и Object Storage поднять в тесте нечем;
  • operations: реальный профиль нагрузки. Проект работает на единицах записей в день, и утверждения о росте остаются условиями, а не замерами;
  • security: стойкость ffmpeg к вредоносному входу — разбор чужого формата отдан внешней программе, и она вне нашей границы.

Перестали проверять сознательно:

  • по записи в docs/review.md — «Ничего не отключали: проверять пока и не начинали». Однако этим изменением список пополняется фактически: приём перестал проверяться сквозь настоящий ffprobe (решение записано в design.md § Risks и обосновано — прежняя проверка проверяла отказ внешней программы и выдавала его за проверку приёма). Раздел docs/review.md этого ещё не знает; строку туда добавляет синк документации, не я.

Сверх записанного в проекте — общее, чего не видит ни один прогон: история инцидентов, поведение под реальным потоком, поведение внешних систем в их версиях, завязка потребителей на текущее поведение и вопрос «а нужна ли эта функциональность вообще».

Каких документов проекта не хватило

Строкой на каждый, с причиной — деградация поразрядная:

  • docs/conventions/ про тесты файла нет: конвенции покрывают конфиг, базу, ошибки, журнал и веб-UI. Изменение целиком про тесты, и сверять его форму было не с чем — отсюда promote-кандидат №1, а не находка.
  • docs/adr/ пуст: только README.md и template.md, ни одного решения. См. обязательную строку 1 ниже.
  • docs/research/ — только README.md, записанных замеров нет. См. строку 2.
  • docs/review.md § «Как настроен конвейер» устарел с этого прогона: там написано «Конвейера ревью в проекте пока нет: плагин не подключён, ни одного прогона не было». Прогон был — этот. Отсев ложноположительных при этом не был слепым: раздел «Типовые ложноположительные» заполнен наперёд, четыре пункта, и я им пользовался.
  • docs/security.md в проекте есть, но на метке small план отправил тему security в инварианты CLAUDE.md, и как дом темы security.md не открывался. См. строку 5 ниже.

Четыре строки, которых не принесёт ни один проход

  1. Решения проекта не сверялись. docs/adr/* — процессный документ, прогон его не открывает. Расхождение изменения с записанным решением ловит сверка документации (скилл av-dev-docs:healthcheck), а не ревью. Здесь у этого есть и вторая сторона: каталог решений пуст, сверять было бы не с чем.
  2. Записанные наблюдения проекта не использовались. docs/research/ — тоже процессный. Всякое число в этом отчёте снято командой на этом прогоне; чисел без приложенной команды в отчёте нет.
  3. Поимённая сверка с руководствами по стилю Go не задавалась ни одним проходом. Различение «идиоматично против просто распространено» на этом прогоне не спрашивал никто.
  4. Альтернативной реализации, с которой можно сдиффить решения, у конвейера нет. Проход независимой реализации снят по стоимости, а не по замеру. «Не знаю, чего не знаю» здесь никто не достаёт: например, вопрос «а верна ли сама форма подстановки в сборке теста» не задал никто, кроме автора дизайна.

Пятая строка — следствие метки small

Темы security, operations и architecture сверялись только с записанными инвариантами CLAUDE.md; дома этих тем (docs/security.md, docs/architecture.md, docs/conventions/logging.md как источник норм журнала) не открывались. Свойство, которого нет в семи пунктах инвариантов, на этом прогоне не проверил никто.

Отдельно про корректор метки

review-code метку small подтвердил, сигнала о занижении не подал. review-basics не запускался, поэтому второго, независимого от review-code подтверждения метки нет. Согласия двух проходов здесь не было бы и при запуске: несколько агентов — один источник, высказавшийся несколько раз; совпадение подняло бы приоритет, но не confidence.