fix(billing): право на прогон не должно сгорать за наш счёт - #9
Merged
Conversation
Первый живой пользователь бота сжёг единственный бесплатный прогон на нашем сбое: Anthropic отдавал 529 семь минут (30 отказов), ход падал, человек трижды увидел "Упс" и нажал "начать заново" — delete_session удалял сессию без возврата права → пейволл 100 звёзд. Он оплатил нашу поломку. Модель: списываем на старте (атомарность оставляем), возвращаем, если spec.md не выдан. Отсюда несущий инвариант — строку сессии НЕЛЬЗЯ удалять, она реестр списания: DELETE уносит защиту от повторного возврата. reset помечает abandoned, транскрипт выживает. План пережил два круга ревью и обе версии были сломаны: - v1: порядок блокировок обратен собственной прозе (ABBA-дедлок); артефакт-гард не ловил гонку, ради которой вводился, и отказывал в возврате на сценарии самого инцидента; бюджет ретраев множился на tool-loop в ~48 минут. - v2: условный finish закрывал гонку, но открывал худшее — статус коммитится ДО доставки, так что упавший on_document оставлял человека без спеки, без возврата и с пейволлом, а новый гард делал спеку недостижимой навсегда. Тот же инцидент, сдвинутый на 30 секунд вправо. v3: статус гейтит деньги, а не выдачу (finish идемпотентен + /spec); дедлайн перед КАЖДОЙ попыткой с клампом таймаута; ретраим весь 5xx (max_retries=0 у SDK иначе роняет 500/502/503); log.error вместо warning — при event_level=ERROR warning дал бы breadcrumb, а не issue. Платные каналы сегодня не работают: fusion вернул набивку пробелами, council дважды INSUFFICIENT по таймаутам. Обе критические находки — от бесплатного ревьюера, подтверждены по коду. Co-Authored-By: Claude <noreply@anthropic.com>
…а спеки Тесты до реализации. Падают по правильным причинам: AttributeError: abandon_session — метода нет (10 тестов) StepError: сессия уже завершена — ровно баг C-1 ImportError: ClaudeOverloaded — класса нет Каждый тест кодирует то, за что уже заплатил живой человек: - reset возвращает прогон и не ведёт в пейволл (инцидент 2026-07-17); - двойной reset не печатает бесплатные прогоны; - spec.md выдан → возврата нет; - транскрипт переживает reset (наша единственная поверхность отладки); - платный прогон возвращается платным, не бесплатным; - владельцу не врём про «вернулся на счёт» — ему нечего возвращать; - finish после abandon не воскрешает сессию (иначе спека + возврат = бесплатно); - 529/429/503/asyncio.TimeoutError ретраятся с растущими паузами ≥30с; - 400/401/403 не ретраятся — ожиданием не чинятся; - дедлайн истёк → не спать и не слать; - SDK max_retries=0 — иначе попытки умножаются (было ~90 запросов на ход); - спека переотдаётся после провала доставки (C-1: иначе недостижима навсегда). Два теста зелёные сразу — это гарды: assemble до закрытия статуса и запрет финиша недоделанного пайплайна. Фиксируют правильное, чтобы не сломать. Co-Authored-By: Claude <noreply@anthropic.com>
…грузку Claude
Первый живой пользователь сжёг единственный бесплатный прогон на НАШЕМ сбое:
Anthropic отдавал 529 семь минут, ход падал, человек трижды увидел "Упс", нажал
"начать заново" — delete_session удалял сессию без возврата → пейволл 100⭐.
Строка сессии стала РЕЕСТРОМ списания (миграция 006: consumed/refunded), отсюда
несущий инвариант: её нельзя удалять — DELETE уносит защиту от повторного
возврата. abandon_session помечает 'abandoned' и возвращает право в ТУ ЖЕ
корзину; идемпотентность — в условии UPDATE, не в проверке кода (урок
pending-invoice-uniqueness). Транскрипт выживает — бонусом вернулась поверхность
отладки.
Порядок блокировок billing→sessions одинаков в обоих методах и закреплён
комментарием: обратный порядок = ABBA-дедлок (я допустил его в плане v1).
finish стал идемпотентным (C-1, находка ревью — опаснее исходного бага): статус
коммитился ДО доставки, поэтому упавший on_document оставлял человека без спеки,
без возврата и с пейволлом, а старый гард "сессия уже завершена" делал спеку
недостижимой НАВСЕГДА. Теперь статус гейтит деньги, а не выдачу: повторный finish
пересобирает и отдаёт (assemble детерминирован, прогон оплачен), + команда /spec
как путь для того, у кого доставка упала. Воскрешение брошенной сессии закрыто
условным set_status_if_active.
Ретраи: max_retries=0 у SDK (иначе попытки умножались — ~90 запросов на ход),
свой backoff 1-2-4-8-16 с джиттером ТОЛЬКО вверх (симметричный утаскивал сумму до
23с и делал обещание "≥30с" правдой через раз — поймал собственный тест), дедлайн
на весь ход с клампом таймаута, ретраим весь 5xx + 429 + asyncio.TimeoutError
(его поднимает wait_for, не anthropic; забыть = тихая регрессия).
log.error, не warning: event_level=ERROR по умолчанию — warning дал бы breadcrumb,
а не issue, и мы ослепли бы ровно на этом классе.
test_double_finish_rejected ЗАКРЕПЛЯЛ баг C-1 — ожидание изменено осознанно:
защищать надо от воскрешения брошенной сессии, а не от повторной выдачи. Три
теста жгли прогон через delete_session; переведены на set_status('finished'),
иначе возврат сделал бы их зелёными враньём и покрытие пейволла испарилось бы.
182 зелёных, включая живой Postgres: гонка двойного reset, отсутствие дедлока,
паритет репо.
Co-Authored-By: Claude <noreply@anthropic.com>
…возврат Ревью реализации нашло, что я написал дедлайн и НЕ подключил его: turn.py звал claude.turn(system, history, version) без deadline → в проде всегда None → все ветки бюджета мёртвый код → 6 попыток x 90с x 6 итераций tool-loop = ~57 минут молчания. Хуже 48 минут v1, которые я сам же забраковал. Мой тест этого не ловил, потому что передавал deadline в клиент НАПРЯМУЮ, минуя проводку — проверял деталь, а не то, что она включена. turn_budget_s вообще существовал только в FakeSettings тестов: поле-декорация, которое никто не читал. Теперь оно в config.py, а отдельный тест стережёт его наличие в реальных Settings. Остальное из ревью: - MAX_SLEEP_S: Retry-After: 3600 усыпил бы бота на час (кламп независимо от дедлайна); - /spec был написан, но невидим — добавлен в меню команд, а провал доставки теперь говорит "пришли /spec", а не "Упс" (C-1 был закрыт наполовину: спека в БД, а человек об этом не знает → пейволл); - on_text больше не подставляет lite молча: после возврата одно "ок" списывало прогон заново на версии, которую человек не выбирал — возврат обнулялся за одно сообщение. Нет версии → клавиатура, а не списание; - маркер [перегрузка, ответа не было] в транскрипт: иначе два user подряд стали бы штатной картиной и наш дешёвый детектор ослеп бы ровно на классе инцидента; - рассинхрон реестра теперь raise, а не тихий return: возврат False коммитил abandoned и право сгорало навсегда — то самое, что мы чиним; - advance_step/save_and_advance не пишут в брошенную сессию; - 10 двойников claude.turn приняли **_ — иначе следующая правка клиента красит пол-набора не по делу; - get_running_loop, удалена мёртвая настройка claude_max_retries. test_finish_after_abandon был зелёным не по той причине: брал фикстуру orch и ни разу не звал — проверял repo напрямую. Теперь гоняет реальный сценарий через orch.finish. CLAUDE.md: инварианты 11-13 (реестр сессии, право не сгорает за наш счёт, дедлайн на ход) — каждый закрыт регресс-тестом. 186 зелёных, включая живой Postgres. Co-Authored-By: Claude <noreply@anthropic.com>
Настройка удалена из config.py (клиент её не читает — backoff зашит), но осталась в FakeSettings: двойник дрейфует от реальных настроек, а именно на таком дрейфе мы уже обожглись (turn_budget_s жил только в тестах). Co-Authored-By: Claude <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c5bd4f040
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+103
to
+106
| ra = _retry_after(last_exc) if last_exc else None | ||
| if ra is not None: | ||
| delay = ra | ||
| delay = min(delay, MAX_SLEEP_S) # belt-and-braces: never hang on Retry-After |
There was a problem hiding this comment.
When Anthropic returns a Retry-After longer than the remaining turn budget (for example, 3600 seconds with 120 seconds remaining), this cap converts it to 30 seconds before the budget comparison, so the client sleeps briefly and sends another request despite the server explicitly asking it not to. Compare the raw server delay with remaining first and raise ClaudeOverloaded when it cannot be honored; otherwise long overload windows are still hammered.
Useful? React with 👍 / 👎.
Comment on lines
101
to
+105
| res = await con.execute( | ||
| "UPDATE sessions SET current_step=$3, updated_at=now() " | ||
| "WHERE id=$1 AND current_step=$2", | ||
| # AND status='active': an in-flight turn must not keep walking a session | ||
| # the human already abandoned (its money is refunded and gone) | ||
| "WHERE id=$1 AND current_step=$2 AND status='active'", |
There was a problem hiding this comment.
When /reset races with an in-flight save_artifact, the artifact upsert above is committed even though this status-guarded update affects zero rows. Orchestrator.save_artifact ignores the false result and dispatch reports success, so the supposedly abandoned session is mutated and the old turn continues producing notices or questions after reset. Guard the artifact write itself or raise so the transaction rolls back when the session is no longer active.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Зачем
Первый живой пользователь бота сжёг единственный бесплатный прогон на нашем сбое. Хронология подтверждена логами и БД:
free_used=1— право списано на старте529— 30 отказов за 7 минутdelete_sessionбез возврата → пейволл 100⭐Он оплатил нашу поломку. Худший возможный первый опыт.
Что сделано
Модель: списываем на старте (атомарность оставлена — она защищает от двойного списания), возвращаем, если
spec.mdне выдан.Несущий инвариант: строку сессии нельзя удалять — она реестр списания (
consumed/refunded).DELETEуносит защиту от повторного возврата, а каскадом — транскрипт.resetпомечаетabandoned. Идемпотентность — в условииUPDATE, не в проверке кода (урокpending-invoice-uniquenessиз нашей KB).Порядок блокировок
billing → sessionsодинаков в обоих методах и закреплён комментарием — обратный даёт ABBA-дедлок.Ретраи:
max_retries=0у SDK (иначе попытки умножались — ~90 запросов на ход), свой backoff 1-2-4-8-16 с джиттером только вверх, дедлайн на весь ход, весь5xx+429+asyncio.TimeoutError. Честный текст «Claude перегружен» вместо «сбойнуло»,log.error— иначе Sentry ослеп бы на этом классе.План был сломан дважды, реализация — один раз
Ревью ловило меня на одном и том же: я закрывал дыру и открывал соседнюю.
finishзакрывал гонку, но статус коммитится до доставки — упавшийon_documentоставлял человека без спеки, без возврата и с пейволлом, а новый гард делал спеку недостижимой навсегда. Тот же инцидент, сдвинутый на 30 секунд.deadline=None, все ветки бюджета мёртвый код, 57 минут молчания. Мой тест это пропустил, потому что передавал дедлайн в клиент напрямую, минуя проводку.Отсюда: статус гейтит деньги, а не выдачу —
finishидемпотентен,/specпереотдаёт спеку.Приёмка
9 из 10 критериев pass, 186 тестов зелёные (включая живой Postgres: гонка двойного возврата, ABBA-проба, паритет репо). Критерий 10 — операционный, закрыт вне кода.
Изменено ожидание теста (осознанно, вслух):
test_double_finish_rejected→test_second_finish_redelivers_the_same_spec. Он закреплял баг C-1: требование «не отдавать дважды» и делало провал доставки вечным.Сделано в проде (заказано явно)
launch11--production--OWNER-IDS— пострадавший добавлен в безлимит; значение проверено нашим же парсером, а не на глаз. Применится с деплоем этого PR (keyvaultrefрезолвится на старте ревизии).billing.free_usedпострадавшего возвращён1 → 0— прогон, сожжённый нашим сбоем.Инварианты
CLAUDE.md 11–13: реестр сессии, право не сгорает за наш счёт, дедлайн на ход. Каждый закрыт регресс-тестом.
Не проверено
Живой платёж звёздами (как и раньше).
tg/bot.pyне покрыт тестами — предсущее, критерии 1 и 8 упираются в него и верифицированы чтением кода + сценарием черезhandle_incoming.🤖 Generated with Claude Code