lekss361/gendesign

Fork 0

fix(ptica): resume_geo_job больше не возобновляет что попало (#2464) #2946

Merged

bot-backend merged 2 commits from fix/2464-resume-geo-job-guard into main

2026-08-20 06:59:56 +00:00

bot-backend commented

2026-08-19 17:38:55 +00:00

Collaborator

Соседняя ручка фильтрует, эта — нет

cancel_geo_job строкой выше:

UPDATE nspd_geo_jobs SET status='cancelled', ...
 WHERE job_id = :id AND status IN ('queued','running','paused')

resume_geo_job — без фильтра вовсе:

UPDATE nspd_geo_jobs SET status='queued', error=NULL WHERE job_id=:id

При том что докстринг обещает «Re-enqueue paused/failed job».

Следствия:

завершённую задачу можно перевести обратно в queued и прогнать заново, затирая результат;
уже бегущую — поставить в очередь второй раз: два воркера на один job_id, лишние запросы к НСПД, у которого WAF.

Замер

nspd_geo_jobs:  done 61 · cancelled 5     (всего 66)

Все задачи в терминальных статусах. То есть resume на любую существующую делал ровно то, чего не должен.

Второе: ответ всегда был `resumed: true`

Независимо от того, изменилось ли что-нибудь. Теперь ответ отражает факт:

{"job_id": 1, "resumed": false, "status": "done",
 "reason": "статус 'done' не подлежит возобновлению"}

Задача при этом не ставится в очередь. Без этой части правка была бы половинчатой: guard бы стоял, а вызывающий всё равно думал бы, что возобновил.

cancelled оставлен возобновляемым намеренно — cancel это ручное действие оператора, и без такой возможности отменённая по ошибке задача не восстанавливалась бы никак. Докстринг приведён в соответствие.

Проверка

тест	`origin/main`
UPDATE несёт фильтр статуса	красный: `AssertionError: UPDATE без фильтра статуса — возобновляется что угодно`
`done` → не возобновляем и говорим почему	красный
задачи нет → внятная причина	красный
ответ несёт новый ключ `status`	красный
контроль: `paused` возобновляется	зелёный

Контроль пришлось переделать. Первая версия проверяла и resumed, и новый ключ status — и падала на origin/main с KeyError: 'status', то есть по причине «в ответе нет поля», а не «законный путь сломан». Контроль обязан быть зелёным по обе стороны, иначе он не контроль. Разделил.

pytest tests/api/v1: 359 passed, 1 skipped, rc=0

Refs #2464

## Соседняя ручка фильтрует, эта — нет `cancel_geo_job` строкой выше: ```sql UPDATE nspd_geo_jobs SET status='cancelled', ... WHERE job_id = :id AND status IN ('queued','running','paused') ``` `resume_geo_job` — без фильтра вовсе: ```sql UPDATE nspd_geo_jobs SET status='queued', error=NULL WHERE job_id=:id ``` При том что докстринг обещает «Re-enqueue **paused/failed** job». Следствия: - завершённую задачу можно перевести обратно в `queued` и прогнать заново, затирая результат; - уже бегущую — поставить в очередь второй раз: два воркера на один `job_id`, лишние запросы к НСПД, у которого WAF. ## Замер ``` nspd_geo_jobs: done 61 · cancelled 5 (всего 66) ``` Все задачи в терминальных статусах. То есть `resume` на **любую** существующую делал ровно то, чего не должен. ## Второе: ответ всегда был `resumed: true` Независимо от того, изменилось ли что-нибудь. Теперь ответ отражает факт: ```json {"job_id": 1, "resumed": false, "status": "done", "reason": "статус 'done' не подлежит возобновлению"} ``` Задача при этом **не** ставится в очередь. Без этой части правка была бы половинчатой: guard бы стоял, а вызывающий всё равно думал бы, что возобновил. `cancelled` оставлен возобновляемым намеренно — `cancel` это ручное действие оператора, и без такой возможности отменённая по ошибке задача не восстанавливалась бы никак. Докстринг приведён в соответствие. ## Проверка | тест | `origin/main` | |---|---| | UPDATE несёт фильтр статуса | **красный**: `AssertionError: UPDATE без фильтра статуса — возобновляется что угодно` | | `done` → не возобновляем и говорим почему | **красный** | | задачи нет → внятная причина | **красный** | | ответ несёт новый ключ `status` | **красный** | | **контроль**: `paused` возобновляется | **зелёный** | Контроль пришлось переделать. Первая версия проверяла и `resumed`, и новый ключ `status` — и падала на `origin/main` с `KeyError: 'status'`, то есть по причине «в ответе нет поля», а не «законный путь сломан». Контроль обязан быть зелёным по обе стороны, иначе он не контроль. Разделил. `pytest tests/api/v1`: **359 passed, 1 skipped, rc=0** Refs #2464

bot-backend added 1 commit 2026-08-19 17:38:55 +00:00

fix(ptica): resume_geo_job больше не возобновляет что попало (#2464 )

CI Trade-In / changes (pull_request) Successful in 7s

Details

CI / changes (pull_request) Successful in 10s

Details

CI Trade-In / backend-tests (pull_request) Has been skipped

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Failing after 2m19s

Details

CI / backend-tests (pull_request) Successful in 17m5s

Details

af4d2a1853

UPDATE шёл БЕЗ фильтра статуса — в отличие от соседнего cancel_geo_job, который
фильтрует явно (`AND status IN ('queued','running','paused')`):

    UPDATE nspd_geo_jobs SET status='queued', error=NULL WHERE job_id=:id

Следствия: завершённую задачу можно было перевести обратно в 'queued' и прогнать
заново, затирая результат; уже бегущую — поставить в очередь второй раз, получив
двух воркеров на один job_id и лишние запросы к НСПД, у которого WAF.

Замер на проде 19.08: все 66 задач в терминальных статусах (61 done, 5 cancelled).
То есть resume на ЛЮБУЮ существующую делал ровно то, чего не должен.

Второе: ручка возвращала resumed=True всегда, независимо от того, изменилось ли
что-нибудь. Теперь ответ отражает факт — не подошёл статус, значит resumed=False,
текущий статус и причина в ответе, задача НЕ ставится в очередь.

'cancelled' оставлен возобновляемым намеренно: cancel — ручное действие оператора,
и без этого отменённая по ошибке задача не восстанавливалась бы никак.

Тесты: 4 красных на origin/main, главный — «AssertionError: UPDATE без фильтра
статуса — возобновляется что угодно».

Контроль пришлось переделать: первая версия проверяла и новый ключ `status`, из-за
чего падала на origin/main с KeyError, то есть по причине «в ответе нет поля», а не
«законный путь сломан». Разделено: контроль смотрит только resumed и зелёный по обе
стороны, новый ключ проверяется отдельным тестом.

pytest tests/api/v1: 359 passed, 1 skipped, rc=0

Light1YT added 1 commit 2026-08-20 06:03:32 +00:00

fix(ptica): cancel_geo_job перестаёт подтверждать отмену, которой не было (#2464 )

CI Trade-In / changes (pull_request) Successful in 6s

Details

CI / changes (pull_request) Successful in 9s

Details

CI Trade-In / backend-tests (pull_request) Has been skipped

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Successful in 1m12s

Details

CI / openapi-codegen-check (pull_request) Successful in 2m5s

Details

CI / backend-tests (pull_request) Successful in 17m13s

Details

12a1a6fd2c

Близнец только что исправленного resume: фильтр статуса у cancel был всегда, но
ответ возвращал cancelled=True независимо от того, задел ли UPDATE хоть одну
строку. Несуществующий job_id и уже завершённая задача давали тот же ответ, что
настоящая отмена — оператор и админ-UI получали подтверждение действия, которого
не произошло.

Теперь ответ отражает факт: cancelled=False, текущий статус и причина.

Обоснование обеих правок перенесено из ДОКСТРИНГОВ в комментарии. Причину нашёл
gate openapi-codegen-check: FastAPI кладёт докстринг в OpenAPI-description, оттуда
он попадает в опубликованный контракт и в сгенерированные типы фронта. Внутренние
замеры («все 66 задач в терминальных статусах», номера задач) в публичной схеме не
нужны — это утечка внутренней кухни в контракт, и упавший чек поймал её честно.

После переноса в схеме осталось единственное изменение — однострочное описание
resume_geo_job. Оно верное: старое обещало «paused/failed», новое описывает
фактическое поведение. api-types.ts перегенерирован тем же способом, что в CI
(openapi-typescript + project-local prettier 3.9.0), и после правок pre-commit
регенерация повторена — схема не сдвинулась.

Тесты cancel: 2 красных на origin/main с `assert True is False`; контроль
(законная отмена running) зелёный по обе стороны — новый ключ status он намеренно
не трогает, иначе падал бы там с KeyError, то есть по причине «в ответе нет поля».

pytest tests/api/v1: 362 passed, 1 skipped, rc=0 — прогон на ИТОГОВОМ дереве.

bot-backend commented

2026-08-20 06:03:54 +00:00

Author

Collaborator

Красный `openapi-codegen-check` был прав — и указал на настоящий дефект

Чек упал не из-за формы ответа (она dict[str, Any], в схеме это объект без свойств), а из-за моих длинных докстрингов.

FastAPI кладёт докстринг в description схемы OpenAPI, оттуда он попадает в опубликованный контракт и в сгенерированные типы фронта (frontend/src/lib/api-types.ts). То есть в публичную схему API утекало внутреннее обоснование: «замер на проде 19.08: все 66 задач в терминальных статусах», ссылки на номер эпика.

Это плохо независимо от гейта. Перенёс обоснование в комментарии над телом функции; в докстринге осталась одна строка — та, что действительно описывает поведение ручки.

После переноса в схеме осталось одно изменение — однострочное описание resume_geo_job. Оно верное: старое обещало «paused/failed», новое описывает фактическое поведение. api-types.ts перегенерирован тем же способом, что в CI, и регенерация повторена после правок pre-commit — схема не сдвинулась.

Второй коммит: близнец у `cancel_geo_job`

Разбирая resume, я приводил cancel_geo_job в пример — у него фильтр статуса был всегда. И не заметил, что у него та же вторая половина дефекта: ответ возвращал cancelled=True независимо от того, задел ли UPDATE хоть одну строку. Несуществующий job_id и уже завершённая задача давали тот же ответ, что настоящая отмена.

Правка в этой же ветке, а не отдельным PR: файл один и тот же, параллельные PR на общие файлы в репозитории запрещены.

Тесты: 2 красных на origin/main с assert True is False, контроль зелёный по обе стороны.

pytest tests/api/v1: 362 passed, 1 skipped, rc=0 — на итоговом дереве.

### Красный `openapi-codegen-check` был прав — и указал на настоящий дефект Чек упал не из-за формы ответа (она `dict[str, Any]`, в схеме это объект без свойств), а из-за **моих длинных докстрингов**. FastAPI кладёт докстринг в `description` схемы OpenAPI, оттуда он попадает в опубликованный контракт и в сгенерированные типы фронта (`frontend/src/lib/api-types.ts`). То есть в публичную схему API утекало внутреннее обоснование: «замер на проде 19.08: все 66 задач в терминальных статусах», ссылки на номер эпика. Это плохо независимо от гейта. Перенёс обоснование в комментарии над телом функции; в докстринге осталась одна строка — та, что действительно описывает поведение ручки. После переноса в схеме осталось **одно** изменение — однострочное описание `resume_geo_job`. Оно верное: старое обещало «paused/failed», новое описывает фактическое поведение. `api-types.ts` перегенерирован тем же способом, что в CI, и регенерация повторена **после** правок pre-commit — схема не сдвинулась. ### Второй коммит: близнец у `cancel_geo_job` Разбирая resume, я приводил `cancel_geo_job` в пример — у него фильтр статуса был всегда. И не заметил, что у него та же вторая половина дефекта: ответ возвращал `cancelled=True` независимо от того, задел ли UPDATE хоть одну строку. Несуществующий job_id и уже завершённая задача давали тот же ответ, что настоящая отмена. Правка в этой же ветке, а не отдельным PR: файл один и тот же, параллельные PR на общие файлы в репозитории запрещены. Тесты: 2 красных на `origin/main` с `assert True is False`, контроль зелёный по обе стороны. `pytest tests/api/v1`: **362 passed, 1 skipped, rc=0** — на итоговом дереве.