From 9cc3f5e950e039cf4e25a0dcc246fb149e86c253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Tue, 28 Jul 2026 06:36:11 +0300 Subject: [PATCH] =?UTF-8?q?fix=20=D1=80=D0=B5=D0=BA=D0=BB=D0=B0=D0=BC?= =?UTF-8?q?=D0=B0=20=D0=B7=D0=B0=20=D0=BF=D0=BE=D0=BA=D0=B0=D0=B7=D1=8B:?= =?UTF-8?q?=20=D0=BF=D0=B0=D1=83=D0=B7=D0=B0=20=D0=BD=D0=B5=20=D0=B2=D1=80?= =?UTF-8?q?=D1=91=D1=82=20=D0=BF=D1=80=D0=BE=20=D0=BE=D1=81=D1=82=D0=B0?= =?UTF-8?q?=D0=BD=D0=BE=D0=B2=D0=BA=D1=83,=20=D1=86=D0=B5=D0=BD=D0=B0=20?= =?UTF-8?q?=D0=B7=D0=B0=D0=BF=D1=83=D1=81=D0=BA=D0=B0=20=D0=BE=D1=81=D1=82?= =?UTF-8?q?=D0=B0=D1=91=D1=82=D1=81=D1=8F=20=D0=BD=D0=B0=20=D0=BA=D0=B0?= =?UTF-8?q?=D0=BC=D0=BF=D0=B0=D0=BD=D0=B8=D0=B8,=20=D1=80=D0=BE=D0=B1?= =?UTF-8?q?=D0=BE=D1=82=20=D0=BD=D0=B5=20=D0=B4=D0=B5=D1=80=D1=91=D1=82?= =?UTF-8?q?=D1=81=D1=8F=20=D1=81=D0=B0=D0=BC=20=D1=81=20=D1=81=D0=BE=D0=B1?= =?UTF-8?q?=D0=BE=D0=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Хвосты денег Д1-Д6 и робота Р-х1-Р-х6 из приёмочного листа v12. Д1 цена, по которой заморожены деньги, записывается на кампанию. Пока поле было пустым, списание читало глобальную цену — админ менял её, и клиент платил больше обещанного при запуске. Д2 суточное списание берёт кампанию под замком строки. Ключ идемпотентности зависит от числа показов, поэтому два одновременных прогона получали разные ключи и списали бы клиента дважды. Д3 пауза, не дошедшая до Директа, больше не считается паузой: отказ 409, заморозка остаётся. Раньше реклама крутилась дальше, портал показывал паузу, а деньги были уже свободны. У возобновления поведение намеренно прежнее — иначе понадобилось бы пятое место разморозки, а их ровно четыре. Там же убрана мина строгого сравнения рубильника. Д4 не трогали — это вопрос владельца. Д5 рубильник Директа держит и служебный канал робота: выдача задания и приём отчёта ходили в живой кабинет мимо него. Д6 проверка рубильника приведена к общему виду: YANDEX_DIRECT_ENABLED=0 давало строку, которую строгое сравнение читало как включено. Р-х1 настройки читаются из .env робота, а не каталога запуска. Р-х2 файл-замок robot.lock: проход и поддержание входа больше не дерутся за профиль браузера. Занят — уходим молча, задание остаётся в очереди. Брошенный замок перехватывается через полчаса. Р-х3 письмо-алярм честно говорит, залиты ли уже креативы в кабинет. Побочно вскрылось, что тексты писем не проверялись ни одним тестом — транспорт вынесен в src/smtp.js. Р-х4 тест-пустышка про рабочую папку заменён настоящим: запуск из чужого каталога без явной папки. Проверено вырезанием. Р-х6 пустое значение в окружении читается как значение по умолчанию, мусор даёт внятную ошибку вместо тихого NaN. Портал 293/293, робот 57/57. Денежных выходов снятия заморозки по-прежнему четыре. Co-Authored-By: Claude Opus 5 --- .../Api/AdvertisingCampaignController.php | 42 ++++++-- app/app/Jobs/ChargeCampaignSpendJob.php | 9 +- .../Services/Advertising/CampaignLauncher.php | 6 ++ .../Advertising/CreativeJobService.php | 7 ++ .../AdvertisingCampaignEndpointTest.php | 33 +++++++ .../AdvertisingCampaignPauseResumeTest.php | 78 ++++++++++++++- .../Advertising/CampaignLauncherTest.php | 31 ++++++ .../ChargeCampaignSpendJobTest.php | 36 +++++++ .../Advertising/CreativeJobServiceTest.php | 20 ++++ bots/yandex-creatives/.gitignore | 1 + bots/yandex-creatives/README.md | 13 +++ bots/yandex-creatives/bin/keepalive.js | 32 ++++-- bots/yandex-creatives/bin/login.js | 5 +- bots/yandex-creatives/bin/run.js | 40 ++++++-- bots/yandex-creatives/src/config.js | 23 ++++- bots/yandex-creatives/src/env.js | 15 +++ bots/yandex-creatives/src/lock.js | 80 +++++++++++++++ bots/yandex-creatives/src/mailer.js | 42 +++++--- bots/yandex-creatives/src/runner.js | 4 +- bots/yandex-creatives/src/smtp.js | 16 +++ bots/yandex-creatives/test/config.test.js | 22 +++++ bots/yandex-creatives/test/lock.test.js | 95 ++++++++++++++++++ bots/yandex-creatives/test/mailer.test.js | 39 ++++++++ bots/yandex-creatives/test/runner.test.js | 72 +++++++++++++- .../2026-07-27-PROGRESS-pochinka-v12.md | 98 ++++++++++++++++++- 25 files changed, 805 insertions(+), 54 deletions(-) create mode 100644 bots/yandex-creatives/src/env.js create mode 100644 bots/yandex-creatives/src/lock.js create mode 100644 bots/yandex-creatives/src/smtp.js create mode 100644 bots/yandex-creatives/test/lock.test.js create mode 100644 bots/yandex-creatives/test/mailer.test.js diff --git a/app/app/Http/Controllers/Api/AdvertisingCampaignController.php b/app/app/Http/Controllers/Api/AdvertisingCampaignController.php index fbe82aec..bedfa9e7 100644 --- a/app/app/Http/Controllers/Api/AdvertisingCampaignController.php +++ b/app/app/Http/Controllers/Api/AdvertisingCampaignController.php @@ -373,7 +373,16 @@ class AdvertisingCampaignController extends Controller ], 409); } - $this->callDirect($campaign, fn (YandexDirectClient $direct, int $yandexCampaignId) => $direct->suspendCampaign($yandexCampaignId)); + // Пауза, которая не дошла до Директа, — не пауза. Раньше ошибку глотали в журнал, + // ставили статус «на паузе» и БЕЗУСЛОВНО размораживали деньги: реклама в Яндексе + // продолжала крутиться и тратить, портал показывал «на паузе», а деньги за неё уже + // были свободны и могли уйти на другую кампанию. Клиент уходил в минус молча. + $error = $this->callDirect($campaign, fn (YandexDirectClient $direct, int $yandexCampaignId) => $direct->suspendCampaign($yandexCampaignId)); + if ($error !== null) { + return response()->json([ + 'message' => 'Не удалось остановить рекламу в Яндексе — попробуйте ещё раз через минуту. Кампания продолжает работать, деньги под неё зарезервированы.', + ], 409); + } $campaign->update(['status' => AdCampaign::STATUS_PAUSED]); @@ -436,14 +445,23 @@ class AdvertisingCampaignController extends Controller /** * Вызывает Директ (suspend/resume) под рубильником, если у кампании уже есть - * yandex_campaign_id. Деньги не трогает. Если Директ недоступен — логируем и - * всё равно продолжаем менять локальный статус (клиент ждёт паузу/возобновление - * здесь и сейчас, синхронизация с Директом — не блокер). + * yandex_campaign_id. Деньги не трогает. + * + * Возвращает null при успехе (в том числе когда идти в Директ не нужно вовсе) либо + * текст ошибки. Решать, что делать с неудачей, — задача вызывающего: для паузы она + * критична (иначе реклама крутится, а деньги уже разморожены), для возобновления — + * нет: там деньги заморожены заранее, и клиент увидит кампанию работающей, даже если + * до Яндекса мы не достучались. Отказывать на возобновлении опаснее: заморозка уже + * стоит, а снять её обратно можно было бы только новым местом разморозки — а их + * в системе ровно четыре и пятое заводить нельзя. */ - private function callDirect(AdCampaign $campaign, callable $action): void + private function callDirect(AdCampaign $campaign, callable $action): ?string { - if (config('services.yandex_direct.enabled') !== true || $campaign->yandex_campaign_id === null) { - return; + // 🪤 Была проверка `!== true`. Рубильник приходит из env строкой, и «1» в .env + // читалась бы здесь как «выключено»: в Директ мы бы не пошли, а пауза приняла бы + // это за успех и разморозила деньги — при работающей в Яндексе рекламе. + if (! config('services.yandex_direct.enabled') || $campaign->yandex_campaign_id === null) { + return null; } $direct = new YandexDirectClient( @@ -458,7 +476,11 @@ class AdvertisingCampaignController extends Controller 'campaign_id' => $campaign->id, 'error' => $e->getMessage(), ]); + + return $e->getMessage(); } + + return null; } public function storeAd(Request $request, int $id, CreativeValidator $validator): JsonResponse @@ -519,7 +541,11 @@ class AdvertisingCampaignController extends Controller return response()->json(['errors' => $errors], 422); } - if (config('services.yandex_direct.enabled') === false) { + // 🪤 Была проверка `=== false`. Значение приходит из env, и `YANDEX_DIRECT_ENABLED=0` + // в .env даёт СТРОКУ «0»: рубильник считает её выключенным, а строгое сравнение — + // включённым, и запрос уходил бы в живой Яндекс при выключенном рубильнике. + // Остальные места проверяют именно так — приводим к общему виду. + if (! config('services.yandex_direct.enabled')) { return response()->json(['message' => 'Яндекс.Директ выключен — картинку пока не загрузить.'], 409); } diff --git a/app/app/Jobs/ChargeCampaignSpendJob.php b/app/app/Jobs/ChargeCampaignSpendJob.php index 5ac2d4ed..4a908d5a 100644 --- a/app/app/Jobs/ChargeCampaignSpendJob.php +++ b/app/app/Jobs/ChargeCampaignSpendJob.php @@ -77,7 +77,14 @@ class ChargeCampaignSpendJob implements ShouldQueue DB::transaction(function () use ($row, $delivered, $tenantId, $charger, $gate, $stopAll): void { DB::statement('SET LOCAL app.current_tenant_id = '.$tenantId); - $campaign = AdCampaign::where('id', $row->id)->firstOrFail(); + // Замок строки обязателен. Идемпотентность списания держится на ключе + // «yandex-imp:{кампания}:{показы}», а число показов приходит из отчёта + // Директа: два прогона, начавшихся одновременно (ручной запуск поверх + // расписания, повтор упавшей задачи), получат чуть разные числа — значит + // разные ключи, и уникальный индекс по ключу дубль уже не остановит. + // Клиента списали бы дважды. Под замком прогоны выстраиваются в очередь: + // второй увидит уже обновлённый charged_client_rub и спишет только дельту. + $campaign = AdCampaign::where('id', $row->id)->lockForUpdate()->firstOrFail(); $charger->charge($campaign, $delivered); if (! $gate->isSolvent($tenantId)) { diff --git a/app/app/Services/Advertising/CampaignLauncher.php b/app/app/Services/Advertising/CampaignLauncher.php index 30f26cc4..8e763ec5 100644 --- a/app/app/Services/Advertising/CampaignLauncher.php +++ b/app/app/Services/Advertising/CampaignLauncher.php @@ -284,8 +284,14 @@ final class CampaignLauncher // yandex_ad_id на КАМПАНИИ не пишем: объявлений теперь набор, их номера живут на // ad_campaign_banners.yandex_ad_id. Колонка кампании остаётся в базе как // аварийный ручной путь; продуктового кода, который её читает, нет. + // + // client_cpm_rub пишем ОБЯЗАТЕЛЬНО, даже если кампания завелась без своей цены. + // Пока поле пустое, effectiveCpm() читает глобальную цену из настроек рекламы — + // и стоило админу её поменять, как суточное списание шло уже по НОВОЙ, выше + // замороженной. Клиент платил бы больше, чем ему обещали при запуске. $campaign->update([ 'paid_impressions' => $estImpr, + 'client_cpm_rub' => $clientCpm, 'status' => AdCampaign::STATUS_PENDING_MODERATION, 'launched_at' => now(), ]); diff --git a/app/app/Services/Advertising/CreativeJobService.php b/app/app/Services/Advertising/CreativeJobService.php index 7f51af43..f1e64609 100644 --- a/app/app/Services/Advertising/CreativeJobService.php +++ b/app/app/Services/Advertising/CreativeJobService.php @@ -224,6 +224,13 @@ final class CreativeJobService private function client(): YandexDirectClient { + // Рубильник Директа держит и служебный канал робота. Раньше клиент строился + // безусловно: выдача задания и приём отчёта ходили в живой кабинет мимо рубильника — + // ровно то, от чего он и защищает. Проверка та же, что в CampaignLauncher. + if (! config('services.yandex_direct.enabled')) { + throw new RuntimeException('Яндекс.Директ выключен (рубильник yandex_direct.enabled).'); + } + return new YandexDirectClient( (string) config('services.yandex_direct.base_url'), (string) config('services.yandex_direct.token'), diff --git a/app/tests/Feature/Advertising/AdvertisingCampaignEndpointTest.php b/app/tests/Feature/Advertising/AdvertisingCampaignEndpointTest.php index b9457490..fd0c97ab 100644 --- a/app/tests/Feature/Advertising/AdvertisingCampaignEndpointTest.php +++ b/app/tests/Feature/Advertising/AdvertisingCampaignEndpointTest.php @@ -632,6 +632,39 @@ it('answers politely instead of a bare error when the creative job cannot be que expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_DRAFT); }); +/** + * 🪤 Мина той же породы, что уже ловилась в проекте: `config(...) === false`. + * + * Значение приходит из `env('YANDEX_DIRECT_ENABLED', false)`. Стоит написать в `.env` + * `YANDEX_DIRECT_ENABLED=0` — и Laravel вернёт строку «0», которая рубильником читается + * как «выключено», а сравнением `=== false` — как «включено». Запрос ушёл бы в живой + * Яндекс при выключенном рубильнике. Все остальные места проверяют через `! config(...)`. + */ +it('treats a string switch value as off, not on', function () { + config(['services.yandex_direct.enabled' => '0']); + Http::fake(); + + $campaign = AdCampaign::create([ + 'tenant_id' => $this->tenant->id, + 'name' => 'Рубильник строкой', + 'audience_days' => 10, + 'use_uploaded_list' => false, + ]); + $ad = AdCampaignAd::create([ + 'tenant_id' => $this->tenant->id, + 'campaign_id' => $campaign->id, + 'title' => 'Заголовок', + 'text' => 'Текст объявления', + 'href' => 'https://liderra.ru', + ]); + + $this->postJson("/api/advertising/campaigns/{$campaign->id}/ads/{$ad->id}/image", [ + 'file' => UploadedFile::fake()->image('b.jpg', 1080, 607), + ])->assertStatus(409); + + Http::assertNothingSent(); +}); + it('does not touch yandex at all when the direct switch is off', function () { config(['services.yandex_direct.enabled' => false]); Http::fake(); diff --git a/app/tests/Feature/Advertising/AdvertisingCampaignPauseResumeTest.php b/app/tests/Feature/Advertising/AdvertisingCampaignPauseResumeTest.php index f90449a5..c0e73346 100644 --- a/app/tests/Feature/Advertising/AdvertisingCampaignPauseResumeTest.php +++ b/app/tests/Feature/Advertising/AdvertisingCampaignPauseResumeTest.php @@ -3,8 +3,10 @@ declare(strict_types=1); use App\Models\AdCampaign; +use App\Models\AdWallet; use App\Models\Tenant; use App\Models\User; +use App\Services\Advertising\AdWalletService; use Illuminate\Support\Facades\Http; /** @@ -52,6 +54,72 @@ it('pauses a running campaign without calling Direct when disabled', function () ]); }); +/** + * 🪤 Мина со стороны «включено»: рубильник, заданный в .env строкой («1», «true»), + * приходит из env строкой. Проверка `!== true` считала бы такой рубильник ВЫКЛЮЧЕННЫМ и + * молча не шла в Директ, а пауза приняла бы это за успех и разморозила деньги — при том, + * что реклама в Яндексе продолжает крутиться. + */ +it('goes to Direct when the switch is on as a string', function () { + config(['services.yandex_direct.enabled' => '1']); + config(['services.yandex_direct.base_url' => 'https://api-sandbox.direct.yandex.com']); + config(['services.yandex_direct.token' => 'DIRTOKEN']); + Http::fake(['*/json/v5/campaigns' => Http::response(['result' => ['SuspendResults' => [['Id' => 555]]]])]); + + $campaign = AdCampaign::create([ + 'tenant_id' => $this->tenant->id, + 'name' => 'Рубильник строкой', + 'audience_days' => 10, + 'status' => AdCampaign::STATUS_RUNNING, + 'yandex_campaign_id' => 555, + ]); + + $this->postJson("/api/advertising/campaigns/{$campaign->id}/pause")->assertOk(); + + Http::assertSent(fn ($request) => str_contains($request->url(), '/json/v5/campaigns')); +}); + +/** + * Пауза, которая не дошла до Директа, — это не пауза. + * + * Ошибку Директа портал глотал в журнал, ставил кампании статус «на паузе» и БЕЗУСЛОВНО + * размораживал деньги. Итог: реклама в Яндексе продолжает крутиться и тратить, портал + * показывает «на паузе», а деньги за неё уже свободны и могут уйти на другую кампанию. + * Клиент уходит в минус молча. + * + * Честнее отказать: «не удалось остановить, попробуйте ещё раз». + */ +it('refuses to pause and keeps the money frozen when Direct rejects the suspend', function () { + config(['services.yandex_direct.enabled' => true]); + config(['services.yandex_direct.base_url' => 'https://api-sandbox.direct.yandex.com']); + config(['services.yandex_direct.token' => 'DIRTOKEN']); + Http::fake(['*/json/v5/campaigns' => Http::response(['error' => ['error_string' => 'Директ лёг']], 500)]); + + app(AdWalletService::class)->topup($this->tenant->id, '5000.00', 'yandex', 'тест'); + + $campaign = AdCampaign::create([ + 'tenant_id' => $this->tenant->id, + 'name' => 'Пауза не дошла до Яндекса', + 'audience_days' => 10, + 'status' => AdCampaign::STATUS_RUNNING, + 'yandex_campaign_id' => 555, + 'paid_impressions' => 10000, + 'client_cpm_rub' => '120.00', + ]); + + app(AdWalletService::class) + ->freeze($this->tenant->id, 'yandex', 'campaign', (int) $campaign->id, '1200.00'); + + $this->postJson("/api/advertising/campaigns/{$campaign->id}/pause") + ->assertStatus(409); + + // Статус не сменился, деньги остались зарезервированными под работающую рекламу. + expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_RUNNING); + + $wallet = AdWallet::where('tenant_id', $this->tenant->id)->first(); + expect($wallet->frozen_rub)->toBe('1200.00'); +}); + it('pauses a pending_moderation campaign and calls Direct suspend when enabled', function () { config(['services.yandex_direct.enabled' => true]); config(['services.yandex_direct.base_url' => 'https://api-sandbox.direct.yandex.com']); @@ -198,7 +266,7 @@ it('returns 404 resuming another tenant campaign', function () { it('ВЫХОД 4: пауза возвращает заморозку в свободные деньги', function () { config(['services.yandex_direct.enabled' => false]); - $svc = app(App\Services\Advertising\AdWalletService::class); + $svc = app(AdWalletService::class); $svc->topup($this->tenant->id, '3000.00', 'yandex', 'тест'); $campaign = AdCampaign::create([ @@ -215,12 +283,12 @@ it('ВЫХОД 4: пауза возвращает заморозку в своб $this->postJson("/api/advertising/campaigns/{$campaign->id}/pause")->assertOk(); - expect(App\Models\AdWallet::where('tenant_id', $this->tenant->id)->first()->frozen_rub)->toBe('0.00'); + expect(AdWallet::where('tenant_id', $this->tenant->id)->first()->frozen_rub)->toBe('0.00'); }); it('ВЫХОД 4: возобновление снова морозит неоткрученный остаток сметы', function () { config(['services.yandex_direct.enabled' => false]); - $svc = app(App\Services\Advertising\AdWalletService::class); + $svc = app(AdWalletService::class); $svc->topup($this->tenant->id, '3000.00', 'yandex', 'тест'); $campaign = AdCampaign::create([ @@ -235,12 +303,12 @@ it('ВЫХОД 4: возобновление снова морозит неот $this->postJson("/api/advertising/campaigns/{$campaign->id}/resume")->assertOk(); - expect(App\Models\AdWallet::where('tenant_id', $this->tenant->id)->first()->frozen_rub)->toBe('1000.00'); + expect(AdWallet::where('tenant_id', $this->tenant->id)->first()->frozen_rub)->toBe('1000.00'); }); it('ВЫХОД 4: возобновление при нехватке денег отказывает понятно и оставляет паузу', function () { config(['services.yandex_direct.enabled' => false]); - $svc = app(App\Services\Advertising\AdWalletService::class); + $svc = app(AdWalletService::class); $svc->topup($this->tenant->id, '500.00', 'yandex', 'тест'); // меньше остатка 1000 ₽ $campaign = AdCampaign::create([ diff --git a/app/tests/Feature/Advertising/CampaignLauncherTest.php b/app/tests/Feature/Advertising/CampaignLauncherTest.php index 24a8aceb..6dbd46d6 100644 --- a/app/tests/Feature/Advertising/CampaignLauncherTest.php +++ b/app/tests/Feature/Advertising/CampaignLauncherTest.php @@ -254,6 +254,37 @@ it('refuses to launch when the creative number is not found in the cabinet', fun expect($campaign->fresh()->status)->toBe(AdCampaign::STATUS_DRAFT); }); +/** + * Цена, по которой заморозили деньги, обязана остаться НА КАМПАНИИ. + * + * Кампания могла быть заведена без своей цены — тогда `effectiveCpm()` читает глобальную + * из настроек рекламы. Заморозка считается по ней и записывается в кошелёк, а вот на самой + * кампании цена не оставалась. Админ менял глобальную цену — и суточное списание шло уже + * по НОВОЙ, выше замороженной: клиент платил больше, чем ему обещали при запуске. + */ +it('records the client price the money was frozen at', function () { + configureYandex(); + fakeYandexEndpoints(); + + $tenant = Tenant::factory()->create(); + app(AdWalletService::class)->topup($tenant->id, '20000.00', 'yandex', 'тест'); + + // Своей цены у кампании нет — берётся глобальная из ad_settings. + $campaign = makeImpressionCampaign($tenant->id, ['client_cpm_rub' => null]); + seedAudience($campaign, 100); + seedBanners($campaign, ['300x250' => 4242]); + + $globalCpm = $campaign->effectiveCpm(); + + app(CampaignLauncher::class)->launch($campaign); + + expect($campaign->fresh()->client_cpm_rub)->toBe($globalCpm); + + // Админ поменял глобальную цену — запущенная кампания обязана остаться на своей. + DB::table('ad_settings')->update(['client_cpm_rub' => '999.00']); + expect($campaign->fresh()->effectiveCpm())->toBe($globalCpm); +}); + it('throws AudienceTooSmallException and does not freeze when audience is under 100', function () { configureYandex(); fakeYandexEndpoints(); diff --git a/app/tests/Feature/Advertising/ChargeCampaignSpendJobTest.php b/app/tests/Feature/Advertising/ChargeCampaignSpendJobTest.php index 2a48da33..e6f2c231 100644 --- a/app/tests/Feature/Advertising/ChargeCampaignSpendJobTest.php +++ b/app/tests/Feature/Advertising/ChargeCampaignSpendJobTest.php @@ -10,6 +10,7 @@ use App\Models\AdWalletTransaction; use App\Models\Tenant; use App\Services\Advertising\AdWalletService; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Http; use Tests\Concerns\SharesSupplierPdo; @@ -72,6 +73,41 @@ it('charges the client for delivered impressions at the flat CPM (client_cpm_rub ->and($tx->amount_rub)->toBe('-300.00'); }); +/** + * Кампания в суточном списании берётся ПОД ЗАМКОМ строки. + * + * Идемпотентность списания держится на ключе `yandex-imp:{кампания}:{показы}`, а число + * показов приходит из отчёта Директа. Два прогона, начавшихся одновременно (ручной запуск + * поверх расписания, повтор упавшей задачи), получат чуть разные числа показов — значит + * разные ключи, и уникальный индекс по ключу дубль уже не остановит: клиент будет списан + * дважды. Замок строки выстраивает прогоны в очередь: второй увидит уже обновлённый + * `charged_client_rub` и спишет только настоящую дельту. + * + * Настоящую гонку в тесте не поставить, поэтому смотрим на сам запрос: он обязан быть + * блокирующим. Уберите `lockForUpdate()` — тест покраснеет. + */ +it('reads the campaign under a row lock while charging', function () { + configureYandexDirectForSpend(); + fakeYandexImpressionsReport(); + + $tenant = Tenant::factory()->create(); + app(AdWalletService::class)->topup($tenant->id, '1000.00', 'yandex', 'тест'); + $campaign = makeRunningCampaign($tenant->id); + + $locked = false; + DB::listen(function ($query) use (&$locked, $campaign) { + if (str_contains($query->sql, 'ad_campaigns') + && str_contains(strtolower($query->sql), 'for update') + && in_array($campaign->id, $query->bindings, true)) { + $locked = true; + } + }); + + app(ChargeCampaignSpendJob::class)->handle(); + + expect($locked)->toBeTrue(); +}); + it('does not double-charge on a second run (idempotent by external_key)', function () { configureYandexDirectForSpend(); fakeYandexImpressionsReport(); diff --git a/app/tests/Feature/Advertising/CreativeJobServiceTest.php b/app/tests/Feature/Advertising/CreativeJobServiceTest.php index 8d6478b7..86fe356b 100644 --- a/app/tests/Feature/Advertising/CreativeJobServiceTest.php +++ b/app/tests/Feature/Advertising/CreativeJobServiceTest.php @@ -229,6 +229,26 @@ it('fails a done report for a campaign without banners without asking Yandex at expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_FAILED); }); +/** + * Рубильник Директа обязан держать и служебный канал робота. + * + * Очередь заданий строила клиента Директа безусловно: выдача задания и приём отчёта ходили + * в живой Яндекс мимо рубильника. То есть при выключенном рубильнике портал всё равно + * стучался в боевой кабинет — ровно то, от чего рубильник и защищает. + */ +it('does not touch Yandex from the robot channel when the Direct switch is off', function () { + config(['services.yandex_direct.enabled' => false]); + Http::fake(); + + $campaign = makeCampaignWithBanners([[300, 250]]); + app(CreativeJobService::class)->enqueue($campaign); + + expect(fn () => app(CreativeJobService::class)->takeNext()) + ->toThrow(RuntimeException::class, 'выключен'); + + Http::assertNothingSent(); +}); + /** * Робот должен возить только то, чего в кабинете ещё нет. * diff --git a/bots/yandex-creatives/.gitignore b/bots/yandex-creatives/.gitignore index d97fa12e..e7453f78 100644 --- a/bots/yandex-creatives/.gitignore +++ b/bots/yandex-creatives/.gitignore @@ -2,3 +2,4 @@ node_modules/ screenshots/ downloads/ .env +robot.lock diff --git a/bots/yandex-creatives/README.md b/bots/yandex-creatives/README.md index 344c7fee..43c4e116 100644 --- a/bots/yandex-creatives/README.md +++ b/bots/yandex-creatives/README.md @@ -65,6 +65,11 @@ npm test # проверки, браузер не нужен По расписанию: `run:once` — часто, `keepalive` — раз в ~15 минут. +Наложиться друг на друга они не могут: браузерный профиль один, и оба запуска берут общий +файл-замок `robot.lock` в папке робота. Занят замок — процесс молча уходит с кодом 0 и +пробует в следующий раз; задание при этом остаётся в очереди. Замок, брошенный убитым +процессом (перезагрузка сервера), перехватывается через полчаса. + ## Пришло письмо-алярм — что делать В письме указан шаг, на котором робот встал, и приложен снимок экрана. @@ -76,6 +81,14 @@ npm test # проверки, браузер не нужен Задание при этом закрыто как выполненное: правду об успехе знает портал, он сверяет список креативов до и после. Если креативы не появились, портал сам пометит задание сбойным. +🔑 Смотрите на концовку письма — она честно говорит, что происходило в кабинете: + +- **«Робот остановился и ничего в кабинете не менял»** — работа не начиналась, кампания + осталась черновиком, деньги не потрачены. В кабинет идти незачем. +- **«Креативы в кабинет уже загружены»** — робот дошёл до конца, файлы лежат в кабинете. + Загляните туда и в очередь заданий: повторная заливка оставит дубли, а вычистить их + можно только руками. + ## Где почитать подробности - Разметка экранов кабинета, снятая живьём — [docs/cabinet-flow.md](docs/cabinet-flow.md). diff --git a/bots/yandex-creatives/bin/keepalive.js b/bots/yandex-creatives/bin/keepalive.js index 870c4d10..9678c9df 100644 --- a/bots/yandex-creatives/bin/keepalive.js +++ b/bots/yandex-creatives/bin/keepalive.js @@ -1,14 +1,32 @@ -import 'dotenv/config'; +import { loadEnvFile } from '../src/env.js'; import { loadConfig } from '../src/config.js'; import { openBrowser } from '../src/browser.js'; import { isLoggedIn } from '../src/session.js'; +import { acquireLock, robotLockPath } from '../src/lock.js'; + +// Настройки читаем из .env РОБОТА, а не того каталога, откуда его запустило расписание. +loadEnvFile(); // Тихий заход, чтобы вход не заснул. Запускается по расписанию раз в ~15 минут. // Ничего не нажимает, ничего не меняет — только смотрит, жив ли вход. -const config = loadConfig(); -const { context, page } = await openBrowser(config, {}); -const ok = await isLoggedIn(page, config); -await context.close(); -console.log(JSON.stringify({ loggedIn: ok, at: new Date().toISOString() })); -process.exit(ok ? 0 : 1); +// Профиль браузера один, и Chromium держит его под своим замком: второй процесс просто +// не стартует. Проход робота и поддержание входа идут по расписанию и могут наложиться — +// поэтому занят замок, уходим МОЛЧА и с кодом 0: это не сбой, а обычное «сейчас занято». +const lock = acquireLock(robotLockPath()); +if (!lock.ok) { + console.log(JSON.stringify({ skipped: 'робот уже работает', at: new Date().toISOString() })); + process.exit(0); +} + +try { + const config = loadConfig(); + const { context, page } = await openBrowser(config, {}); + const ok = await isLoggedIn(page, config); + + await context.close(); + console.log(JSON.stringify({ loggedIn: ok, at: new Date().toISOString() })); + process.exit(ok ? 0 : 1); +} finally { + lock.release(); +} diff --git a/bots/yandex-creatives/bin/login.js b/bots/yandex-creatives/bin/login.js index f8cd3425..c25e4883 100644 --- a/bots/yandex-creatives/bin/login.js +++ b/bots/yandex-creatives/bin/login.js @@ -1,8 +1,11 @@ -import 'dotenv/config'; +import { loadEnvFile } from '../src/env.js'; import { loadConfig } from '../src/config.js'; import { openBrowser } from '../src/browser.js'; import { isLoggedIn, overviewUrl } from '../src/session.js'; +// Настройки читаем из .env РОБОТА, а не того каталога, откуда его запустило расписание. +loadEnvFile(); + // Разовый заход глазами: на боевом запускается внутри виртуального экрана через // удалённый рабочий стол. Владелец вводит пароль и СМС, профиль остаётся на диске. const config = loadConfig(); diff --git a/bots/yandex-creatives/bin/run.js b/bots/yandex-creatives/bin/run.js index ab123cc3..72f7a309 100644 --- a/bots/yandex-creatives/bin/run.js +++ b/bots/yandex-creatives/bin/run.js @@ -1,20 +1,40 @@ -import 'dotenv/config'; +import { loadEnvFile } from '../src/env.js'; import { loadConfig } from '../src/config.js'; import { createPortal } from '../src/portal.js'; import { openBrowser } from '../src/browser.js'; import { isLoggedIn } from '../src/session.js'; import { uploadCreatives } from '../src/cabinet.js'; -import { createMailer, smtpTransport } from '../src/mailer.js'; +import { createMailer } from '../src/mailer.js'; +import { smtpTransport } from '../src/smtp.js'; import { runOnce } from '../src/runner.js'; +import { acquireLock, robotLockPath } from '../src/lock.js'; + +// Настройки читаем из .env РОБОТА, а не того каталога, откуда его запустило расписание. +loadEnvFile(); // Один проход: взять задание, если оно есть, и довести до конца. Запускается расписанием. -const config = loadConfig(); -const portal = createPortal(config); -const browser = { open: openBrowser, isLoggedIn, uploadCreatives }; -const mailer = createMailer(smtpTransport(config.smtp), { from: config.alarmFrom, to: config.alarmTo }); -const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); -const res = await runOnce(config, portal, browser, mailer, { timestamp }); +// 🔴 Замок берём ДО того, как спросить у портала работу. Профиль браузера один, Chromium +// держит его под своим замком, и второй процесс просто не стартует — а падало это уже +// ПОСЛЕ выдачи задания, и задание помечалось сбойным, хотя ничего не сломано. Занят +// замок — уходим молча и с кодом 0, задание остаётся в очереди до следующего раза. +const lock = acquireLock(robotLockPath()); +if (!lock.ok) { + console.log(JSON.stringify({ skipped: 'робот уже работает' })); + process.exit(0); +} -console.log(JSON.stringify(res, null, 2)); -process.exit(res.idle || res.ok ? 0 : 1); +try { + const config = loadConfig(); + const portal = createPortal(config); + const browser = { open: openBrowser, isLoggedIn, uploadCreatives }; + const mailer = createMailer(smtpTransport(config.smtp), { from: config.alarmFrom, to: config.alarmTo }); + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + + const res = await runOnce(config, portal, browser, mailer, { timestamp }); + + console.log(JSON.stringify(res, null, 2)); + process.exit(res.idle || res.ok ? 0 : 1); +} finally { + lock.release(); +} diff --git a/bots/yandex-creatives/src/config.js b/bots/yandex-creatives/src/config.js index 69e3a718..3896642c 100644 --- a/bots/yandex-creatives/src/config.js +++ b/bots/yandex-creatives/src/config.js @@ -4,6 +4,23 @@ function required(env, key) { return v; } +/** + * Число из окружения. `Number(env.X ?? '800')` ловил только отсутствие переменной: + * пустое значение в .env (`HUMAN_DELAY_MS=`) давало Number('') === 0, и робот начинал + * щёлкать по кабинету с машинной скоростью — ровно так антифрод Яндекса и опознаёт бота. + * Мусор в значении молча становился NaN: пауза «никакая», а порт почты NaN — письма + * переставали уходить без единого внятного слова в журнале. + */ +function number(env, key, fallback) { + const raw = env[key]; + if (raw === undefined || String(raw).trim() === '') return fallback; + + const value = Number(raw); + if (!Number.isFinite(value)) throw new Error(`Переменная окружения ${key} должна быть числом, а там: ${raw}`); + + return value; +} + export function loadConfig(env = process.env) { return { profileDir: required(env, 'YC_BROWSER_PROFILE_DIR'), @@ -22,16 +39,16 @@ export function loadConfig(env = process.env) { robotToken: required(env, 'CREATIVE_ROBOT_TOKEN'), smtp: { host: required(env, 'SMTP_HOST'), - port: Number(required(env, 'SMTP_PORT')), + port: number({ SMTP_PORT: required(env, 'SMTP_PORT') }, 'SMTP_PORT', 0), user: required(env, 'SMTP_USER'), pass: required(env, 'SMTP_PASS'), }, alarmFrom: required(env, 'ALARM_FROM'), alarmTo: required(env, 'ALARM_TO'), // Человекоподобный темп: кабинет не должен видеть машинную скорость. - humanDelayMs: Number(env.HUMAN_DELAY_MS ?? '800'), + humanDelayMs: number(env, 'HUMAN_DELAY_MS', 800), // Пауза между повторами доклада порталу «готово». Доклад повторяется, потому что // обрыв на нём стоит дорого: креативы уже в кабинете, а портал об этом не знает. - reportRetryDelayMs: Number(env.REPORT_RETRY_DELAY_MS ?? '3000'), + reportRetryDelayMs: number(env, 'REPORT_RETRY_DELAY_MS', 3000), }; } diff --git a/bots/yandex-creatives/src/env.js b/bots/yandex-creatives/src/env.js new file mode 100644 index 00000000..348489e2 --- /dev/null +++ b/bots/yandex-creatives/src/env.js @@ -0,0 +1,15 @@ +import { fileURLToPath } from 'node:url'; +import dotenv from 'dotenv'; + +/** + * Читает `.env` РОБОТА, а не того каталога, откуда его запустили. + * + * `import 'dotenv/config'` ищет файл от текущего каталога процесса. Робота запускает + * расписание, и каталог запуска у него может быть любым: настройки просто не нашлись бы, + * а `loadConfig()` упал бы «не задана переменная окружения» — притом что файл лежит + * на месте. Папки для картинок и снимков от каталога запуска отвязаны давно (runner.js), + * настройки оставались последним местом с этой миной. + */ +export function loadEnvFile() { + dotenv.config({ path: fileURLToPath(new URL('../.env', import.meta.url)) }); +} diff --git a/bots/yandex-creatives/src/lock.js b/bots/yandex-creatives/src/lock.js new file mode 100644 index 00000000..66937407 --- /dev/null +++ b/bots/yandex-creatives/src/lock.js @@ -0,0 +1,80 @@ +import { closeSync, openSync, readFileSync, rmSync, writeSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +/** Сколько живёт замок, брошенный убитым процессом. */ +const STALE_MS = 30 * 60 * 1000; + +/** + * Путь к общему замку робота. Считается от корня робота, а не от каталога запуска: + * иначе два запуска из разных мест взяли бы РАЗНЫЕ замки и спокойно подрались бы + * за один и тот же профиль браузера. + */ +export function robotLockPath() { + return fileURLToPath(new URL('../robot.lock', import.meta.url)); +} + +/** + * Файл-замок «работает только один робот». + * + * Зачем. Chromium держит папку профиля под своим замком, и второй процесс просто не + * стартует. Раньше это падало ВНУТРИ рабочего блока — и задание помечалось сбойным, хотя + * ничего не сломано: расписание всего лишь запустило проход поверх ещё не закончившегося + * (README предписывает гонять проход часто, а поддержание входа — раз в ~15 минут). + * Правильное поведение при занятом замке — молча уйти и попробовать в следующий раз. + * + * Обратная сторона: процесс могли убить насмерть (перезагрузка сервера), и снять замок + * было бы некому — робот не работал бы уже никогда. Поэтому замок старше STALE_MS + * считается брошенным и перехватывается. Нечитаемое содержимое (обрыв записи, потрогали + * руками) — тоже: неизвестность не должна запирать робота навсегда. + * + * Атомарность даёт сама файловая система: флаг 'wx' создаёт файл ТОЛЬКО если его нет. + */ +export function acquireLock(path, { staleMs = STALE_MS, pid = process.pid } = {}) { + const taken = tryCreate(path, pid); + if (taken !== null) { + return taken; + } + + if (!isStale(path, staleMs)) { + return { ok: false, release() {} }; + } + + try { rmSync(path, { force: true }); } catch { /* уже убрали */ } + + return tryCreate(path, pid) ?? { ok: false, release() {} }; +} + +/** Возвращает взятый замок либо null, если файл уже существует. */ +function tryCreate(path, pid) { + let fd; + try { + fd = openSync(path, 'wx'); + } catch { + return null; + } + + try { + writeSync(fd, JSON.stringify({ pid, at: Date.now() })); + } finally { + closeSync(fd); + } + + return { + ok: true, + release() { + try { rmSync(path, { force: true }); } catch { /* уже нет */ } + }, + }; +} + +function isStale(path, staleMs) { + try { + const held = JSON.parse(readFileSync(path, 'utf8')); + const at = Number(held?.at); + if (!Number.isFinite(at)) return true; + + return Date.now() - at > staleMs; + } catch { + return true; + } +} diff --git a/bots/yandex-creatives/src/mailer.js b/bots/yandex-creatives/src/mailer.js index 13c061d3..86b38ebb 100644 --- a/bots/yandex-creatives/src/mailer.js +++ b/bots/yandex-creatives/src/mailer.js @@ -1,23 +1,44 @@ -import nodemailer from 'nodemailer'; - /** * Письма человеку. Два повода: «робот встал» и «робот сделал». * - * Транспорт передаётся снаружи — так письма проверяются без настоящей почты. + * Транспорт передаётся снаружи — так письма проверяются без настоящей почты. Настоящий + * транспорт живёт в отдельном файле src/smtp.js: он тянет nodemailer, а зависимости стоят + * только на боевом сервере, и пока импорт был здесь, тексты писем нельзя было проверить + * ни одним тестом. */ export function createMailer(transport, { from, to }) { return { - async alarm({ step, reason, campaignId, screenshotPath }) { + /** + * `uploaded` — были ли креативы к этому моменту УЖЕ залиты в кабинет. + * + * Раньше письмо было одно на все случаи и всегда утверждало «робот остановился и ничего + * в кабинете не менял». Тем же письмом сообщали, например, «окно загрузки не закрылось» — + * а там файлы уже приняты и робот уже отчитался «готово». Человек читал, что ничего + * не произошло, и в кабинет не шёл. А идти надо: там лежат креативы, и повторная заливка + * оставит дубли, которые вычищаются только руками. + */ + async alarm({ step, reason, campaignId, screenshotPath, uploaded = false }) { + const tail = uploaded + ? [ + '🔴 Креативы В КАБИНЕТ УЖЕ ЗАГРУЖЕНЫ — робот дошёл до конца работы.', + 'Загляните в кабинет и в очередь заданий: повторная заливка оставит дубли,', + 'вычистить их можно только руками.', + ] + : [ + 'Робот остановился и ничего в кабинете не менял.', + 'Кампания осталась черновиком, деньги не потрачены.', + ]; + await transport.sendMail({ from, to, subject: `[Робот креативов] АЛЯРМ на шаге «${step}»`, text: [ - 'Робот остановился и ничего в кабинете не менял.', `Кампания: ${campaignId}`, `Шаг: ${step}`, `Причина: ${reason}`, - 'Кампания осталась черновиком, деньги не потрачены.', + '', + ...tail, ].join('\n'), attachments: screenshotPath ? [{ path: screenshotPath }] : [], }); @@ -33,12 +54,3 @@ export function createMailer(transport, { from, to }) { }, }; } - -export function smtpTransport(smtp) { - return nodemailer.createTransport({ - host: smtp.host, - port: smtp.port, - secure: false, - auth: { user: smtp.user, pass: smtp.pass }, - }); -} diff --git a/bots/yandex-creatives/src/runner.js b/bots/yandex-creatives/src/runner.js index 86097a55..47394ba5 100644 --- a/bots/yandex-creatives/src/runner.js +++ b/bots/yandex-creatives/src/runner.js @@ -105,9 +105,10 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work await mailer.alarm({ step: 'доклад порталу', reason: `Креативы загружены в кабинет, но доложить об этом порталу не удалось: ${reportError.message}. ` - + `Задание #${job.id} могло остаться «в работе». Файлы в кабинете УЖЕ ЕСТЬ — повторная заливка оставит дубли.`, + + `Задание #${job.id} могло остаться «в работе».`, campaignId: job.campaign_id, screenshotPath: null, + uploaded: true, }); } catch { /* почта недоступна */ } @@ -127,6 +128,7 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work reason: `Окно загрузки не закрылось. Кабинет сказал: ${said.cabinetSaid || 'ничего не написал'}`, campaignId: job.campaign_id, screenshotPath: null, + uploaded: true, }); } catch { /* см. выше */ } } diff --git a/bots/yandex-creatives/src/smtp.js b/bots/yandex-creatives/src/smtp.js new file mode 100644 index 00000000..82d45d81 --- /dev/null +++ b/bots/yandex-creatives/src/smtp.js @@ -0,0 +1,16 @@ +import nodemailer from 'nodemailer'; + +/** + * Настоящий почтовый транспорт. Вынесен из mailer.js отдельным файлом нарочно — по той же + * причине, что и human.js: `nodemailer` локально не установлен (зависимости ставятся только + * на боевом сервере), и пока он импортировался из mailer.js, тексты писем нельзя было + * проверить ни одним тестом. Теперь mailer.js чистый, а сюда никто, кроме bin/, не ходит. + */ +export function smtpTransport(smtp) { + return nodemailer.createTransport({ + host: smtp.host, + port: smtp.port, + secure: false, + auth: { user: smtp.user, pass: smtp.pass }, + }); +} diff --git a/bots/yandex-creatives/test/config.test.js b/bots/yandex-creatives/test/config.test.js index 4b684973..1e5ad89b 100644 --- a/bots/yandex-creatives/test/config.test.js +++ b/bots/yandex-creatives/test/config.test.js @@ -26,6 +26,28 @@ test('знает паузу между повторами доклада пор assert.equal(loadConfig({ ...full, REPORT_RETRY_DELAY_MS: '500' }).reportRetryDelayMs, 500); }); +// 🪤 `Number(env.X ?? '800')` ловит только undefined. Пустое значение в .env +// (`HUMAN_DELAY_MS=`) даёт пустую строку, а Number('') — это 0: робот начинал бы щёлкать +// по кабинету с машинной скоростью вместо человекоподобного темпа, и именно так выглядит +// поведение бота для антифрода Яндекса. +test('пустое значение паузы в окружении читается как значение по умолчанию, а не как ноль', () => { + assert.equal(loadConfig({ ...full, HUMAN_DELAY_MS: '' }).humanDelayMs, 800); + assert.equal(loadConfig({ ...full, REPORT_RETRY_DELAY_MS: '' }).reportRetryDelayMs, 3000); +}); + +// Мусор в значении молча превращался в NaN: пауза становилась «никакой», а порт почты — +// NaN, и письма переставали уходить без единого внятного слова в журнале. Пустое значение +// и опечатка — разные вещи: пустое значит «не задавал, возьми обычное», опечатка значит +// «человек хотел что-то задать и ошибся». Про второе надо сказать вслух, а не подставлять +// умолчание молча. +test('мусор в значении паузы падает с понятным сообщением', () => { + assert.throws(() => loadConfig({ ...full, HUMAN_DELAY_MS: 'быстро' }), /HUMAN_DELAY_MS/); +}); + +test('нечисловой порт почты падает с понятным сообщением, а не превращается в NaN', () => { + assert.throws(() => loadConfig({ ...full, SMTP_PORT: 'пятьсот' }), /SMTP_PORT/); +}); + test('падает с понятным сообщением, если нет обязательной переменной', () => { const { CREATIVE_ROBOT_TOKEN, ...without } = full; assert.throws(() => loadConfig(without), /CREATIVE_ROBOT_TOKEN/); diff --git a/bots/yandex-creatives/test/lock.test.js b/bots/yandex-creatives/test/lock.test.js new file mode 100644 index 00000000..4e294f79 --- /dev/null +++ b/bots/yandex-creatives/test/lock.test.js @@ -0,0 +1,95 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { existsSync } from 'node:fs'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { acquireLock } from '../src/lock.js'; + +async function withDir(fn) { + const dir = await mkdtemp(join(tmpdir(), 'yc-lock-')); + try { + return await fn(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test('свободный замок берётся, файл замка появляется на диске', async () => { + await withDir(async (dir) => { + const path = join(dir, 'robot.lock'); + + const lock = acquireLock(path); + + assert.equal(lock.ok, true); + assert.equal(existsSync(path), true); + + lock.release(); + assert.equal(existsSync(path), false); + }); +}); + +/** + * Ради чего всё: Chromium держит папку профиля под замком, и второй процесс просто + * не стартует. Раньше это падало ВНУТРИ рабочего блока — и задание помечалось сбойным, + * хотя ничего не сломано: просто расписание запустило проход поверх ещё не закончившегося + * (README предписывает гонять проход часто, а поддержание входа — раз в ~15 минут). + */ +test('занятый замок вторым процессом не берётся', async () => { + await withDir(async (dir) => { + const path = join(dir, 'robot.lock'); + const first = acquireLock(path); + + const second = acquireLock(path); + + assert.equal(first.ok, true); + assert.equal(second.ok, false); + + first.release(); + assert.equal(acquireLock(path).ok, true, 'после освобождения замок снова доступен'); + }); +}); + +/** + * Обратная сторона: процесс убили насмерть (перезагрузка сервера) — снять замок некому, + * и робот не работал бы уже никогда. Поэтому просроченный замок перехватывается. + */ +test('просроченный замок перехватывается', async () => { + await withDir(async (dir) => { + const path = join(dir, 'robot.lock'); + await writeFile(path, JSON.stringify({ pid: 999999, at: Date.now() - 60 * 60 * 1000 })); + + const lock = acquireLock(path, { staleMs: 30 * 60 * 1000 }); + + assert.equal(lock.ok, true); + }); +}); + +test('свежий замок не перехватывается по сроку', async () => { + await withDir(async (dir) => { + const path = join(dir, 'robot.lock'); + await writeFile(path, JSON.stringify({ pid: 999999, at: Date.now() })); + + assert.equal(acquireLock(path, { staleMs: 30 * 60 * 1000 }).ok, false); + }); +}); + +// Замок, испорченный до нечитаемого (обрыв записи, кто-то потрогал руками), не должен +// запирать робота навсегда: непонятное содержимое считаем просроченным. +test('нечитаемый замок не запирает робота навсегда', async () => { + await withDir(async (dir) => { + const path = join(dir, 'robot.lock'); + await writeFile(path, 'мусор, не json'); + + assert.equal(acquireLock(path).ok, true); + }); +}); + +test('повторное освобождение не падает', async () => { + await withDir(async (dir) => { + const lock = acquireLock(join(dir, 'robot.lock')); + + lock.release(); + lock.release(); + }); +}); diff --git a/bots/yandex-creatives/test/mailer.test.js b/bots/yandex-creatives/test/mailer.test.js new file mode 100644 index 00000000..4db529ce --- /dev/null +++ b/bots/yandex-creatives/test/mailer.test.js @@ -0,0 +1,39 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createMailer } from '../src/mailer.js'; + +function stubTransport() { + const sent = []; + + return { sent, sendMail: async (m) => { sent.push(m); } }; +} + +test('алярм до загрузки честно говорит, что в кабинете ничего не изменилось', async () => { + const t = stubTransport(); + + await createMailer(t, { from: 'bot@liderra.ru', to: 'ops@liderra.ru' }) + .alarm({ step: 'проверка входа', reason: 'вход слетел', campaignId: 42, screenshotPath: null }); + + assert.match(t.sent[0].text, /ничего в кабинете не менял/); + assert.match(t.sent[0].text, /деньги не потрачены/); +}); + +/** + * Письмо было одно на все случаи и всегда утверждало «робот остановился и ничего + * в кабинете не менял». А тем же письмом сообщали, например, «окно загрузки не закрылось» — + * там файлы УЖЕ залиты и робот уже отчитался «готово». Человек читал, что ничего + * не произошло, и в кабинет не шёл. А идти надо: там лежат креативы, и повторная заливка + * оставит дубли, которые вычищаются только руками. + */ +test('алярм после загрузки не врёт, что ничего не менялось, и зовёт человека в кабинет', async () => { + const t = stubTransport(); + + await createMailer(t, { from: 'bot@liderra.ru', to: 'ops@liderra.ru' }) + .alarm({ step: 'доклад порталу', reason: 'портал не ответил', campaignId: 42, screenshotPath: null, uploaded: true }); + + const text = t.sent[0].text; + assert.doesNotMatch(text, /ничего в кабинете не менял/); + assert.doesNotMatch(text, /деньги не потрачены/); + assert.match(text, /кабинет/i); + assert.match(text, /дубли/); +}); diff --git a/bots/yandex-creatives/test/runner.test.js b/bots/yandex-creatives/test/runner.test.js index 39732797..263c444c 100644 --- a/bots/yandex-creatives/test/runner.test.js +++ b/bots/yandex-creatives/test/runner.test.js @@ -4,6 +4,7 @@ import { existsSync } from 'node:fs'; import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { runOnce } from '../src/runner.js'; const JOB = { @@ -110,7 +111,7 @@ test('в кабинет уходят ПУТИ к файлам, а не опис }); }); -test('файлы клиента живут в рабочей папке робота, а не там, откуда его запустили', async () => { +test('файлы клиента живут в переданной рабочей папке', async () => { await withWorkDir(async (workDir) => { const s = stubs({ job: JOB }); @@ -120,6 +121,46 @@ test('файлы клиента живут в рабочей папке робо }); }); +/** + * 🪤 Прежний тест на это был ПУСТЫШКОЙ: он всегда передавал рабочую папку явно, поэтому + * значение по умолчанию не проверялось вообще. Проверено вырезанием: подмена умолчания + * на `process.cwd()` оставляла все тесты зелёными. + * + * А умолчание тут и есть вся суть: робота запускает расписание, каталог запуска у него + * может быть любым. Считай мы папки от него — картинки клиента и снимки экрана + * с логином и остатком счёта раскидывало бы по случайным местам сервера. + * + * Поэтому запускаем ИЗ ЧУЖОГО каталога и без явной рабочей папки. + */ +test('без явной рабочей папки файлы всё равно ложатся к роботу, а не в каталог запуска', async () => { + const alien = await mkdtemp(join(tmpdir(), 'yc-alien-')); + const wasCwd = process.cwd(); + const robotRoot = fileURLToPath(new URL('..', import.meta.url)); + const s = stubs({ job: JOB }); + + try { + process.chdir(alien); + + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, { timestamp: 't' }); + + assert.ok(s.downloaded.length > 0, 'файлы должны были скачаться'); + assert.ok( + s.downloaded.every((p) => p.startsWith(robotRoot)), + `файлы легли мимо папки робота: ${s.downloaded[0]}`, + ); + assert.ok( + s.downloaded.every((p) => !p.startsWith(alien)), + `файлы легли в каталог запуска: ${s.downloaded[0]}`, + ); + } finally { + process.chdir(wasCwd); + // Робот чистит downloads сам, а вот папку снимков экрана он создаёт и оставляет. + await rm(join(robotRoot, 'downloads'), { recursive: true, force: true }); + await rm(join(robotRoot, 'screenshots'), { recursive: true, force: true }); + await rm(alien, { recursive: true, force: true }); + } +}); + test('файлы клиента удаляются при любом исходе — это чужие картинки, копиться им нельзя', async () => { await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, uploadThrows: 'кабинет упал' }); @@ -212,6 +253,35 @@ test('доклад «готово» повторяется, если с перв }); }); +// Письмо обязано честно сказать, что в кабинете уже лежат креативы. Пока оно было одно +// на все случаи и всегда утверждало «робот ничего не менял», человек читал письмо +// «окно загрузки не закрылось» и в кабинет не шёл — а идти надо: повторная заливка +// оставит дубли, вычистить которые можно только руками. +test('письма после заливки помечены «креативы уже в кабинете»', async () => { + await withWorkDir(async (workDir) => { + const s = stubs({ job: JOB, uploadResult: { modalClosed: false, cabinetSaid: 'что-то не так' } }); + s.portal.reportDone = async () => { throw new Error('Портал ответил 502 на /done'); }; + + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + const alarms = s.sent.filter((m) => m.kind === 'alarm'); + assert.ok(alarms.length > 0, 'человека надо позвать письмом'); + assert.ok(alarms.every((m) => m.uploaded === true), 'письма после заливки обязаны быть помечены'); + }); +}); + +test('письмо о сбое ДО заливки не помечено «креативы уже в кабинете»', async () => { + await withWorkDir(async (workDir) => { + const s = stubs({ job: JOB, uploadThrows: 'кабинет упал' }); + + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + const alarm = s.sent.find((m) => m.kind === 'alarm'); + assert.ok(alarm); + assert.notEqual(alarm.uploaded, true, 'работа не сделана — врать в обратную сторону тоже нельзя'); + }); +}); + test('портал не принял отчёт о сбое — человек узнаёт об этом из письма', async () => { await withWorkDir(async (workDir) => { // Раньше провал отчёта глотался молча. А это самый опасный исход: задание остаётся diff --git a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md index 42286a00..0fa442f8 100644 --- a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md +++ b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md @@ -299,10 +299,104 @@ `SyncCampaignModerationJob:130`, `AdvertisingCampaignController:383`, `CampaignImpressionCharger:96`); прочие `->release()` — замки `Cache::lock`. +## Хвосты денег Д1–Д6 — закрыты ✅ (кроме Д4 — вопрос владельца) + +### Д1 — на кампанию не записывалась цена, по которой заморожены деньги ✅ + +Кампания могла завестись без своей цены; тогда `effectiveCpm()` читал глобальную из настроек. +Админ менял глобальную цену — и суточное списание шло по НОВОЙ, выше замороженной. Теперь +`client_cpm_rub` пишется в финальном `update()` лаунчера. + +### Д2 — суточное списание читало кампанию без замка строки ✅ + +`ChargeCampaignSpendJob` — `lockForUpdate()`. Идемпотентность держится на ключе +`yandex-imp:{кампания}:{показы}`, а число показов приходит из отчёта Директа: два +одновременных прогона получили бы РАЗНЫЕ ключи, и уникальный индекс дубль не остановил бы. + +Тест смотрит на сам запрос через `DB::listen` (настоящую гонку в тесте не поставить). + +### Д3 — ошибка Директа глоталась, а деньги размораживались всё равно ✅ + +Пауза, не дошедшая до Директа, — не пауза: реклама крутится, портал показывает «на паузе», +деньги свободны. Теперь `callDirect()` возвращает текст ошибки, а `pause()` при неудаче +отдаёт **409** и заморозку не снимает. + +🔴 У `resume()` поведение НЕ менялось намеренно: там заморозка ставится ДО обращения к +Директу, и отказ потребовал бы её снять — то есть **пятое** место разморозки. Их ровно +четыре. Обосновано в докстроке `callDirect()`. + +**Смежная находка:** там же стояло `!== true` — рубильник, заданный в `.env` строкой «1», +читался бы как выключённый: в Директ не пошли бы, а пауза сочла бы это успехом и +разморозила деньги. Приведено к `! config(...)`. + +### Д4 — НЕ ТРОГАЛИ, вопрос владельца + +Как писать проводку при нехватке денег: на фактически списанное или отдельной строкой +«недобор». Влияет на отчёт по марже. Лист v12 §13 п.7. + +### Д5 — рубильник обходился служебным каналом робота ✅ + +`CreativeJobService::client()` строился безусловно: выдача задания и приём отчёта ходили +в живой кабинет мимо рубильника. Добавлена та же проверка, что в лаунчере. + +### Д6 — `=== false` вместо `! config(...)` ✅ + +`YANDEX_DIRECT_ENABLED=0` в `.env` даёт строку «0»: рубильник считает её выключённым, +а строгое сравнение — включённым. **Проверено вырезанием** (первый красный был из-за +опечатки в самом тесте, поэтому проверили отдельно) → красное. + +## Хвосты робота Р-х1–Р-х6 — закрыты ✅ + +### Р-х1 — `.env` читался от каталога запуска ✅ + +Новый `src/env.js` (`loadEnvFile()`) считает путь от корня робота; все три запускалки +переведены на него. + +### Р-х2 — два процесса дрались за профиль браузера ✅ + +Новый `src/lock.js`: файл-замок `robot.lock` в корне робота, атомарный через флаг `wx`. +Занят — процесс уходит **молча с кодом 0** (задание остаётся в очереди), а не падает +внутри рабочего блока с пометкой задания сбойным. Брошенный замок перехватывается через +30 минут; нечитаемый — сразу. Подключён к `bin/run.js` и `bin/keepalive.js`, добавлен +в `.gitignore` и описан в README. + +Тесты: новый `test/lock.test.js` (6 штук). + +### Р-х3 — письмо врало «ничего не менял», когда файлы уже залиты ✅ + +У `alarm()` появился признак `uploaded`. После заливки письмо прямо говорит «креативы +в кабинет уже загружены» и зовёт человека посмотреть кабинет и очередь. + +🔑 Побочно вскрылось: `mailer.js` тянул `nodemailer` верхним импортом, а зависимости +локально не ставились — **тексты писем не проверялись ни одним тестом вообще**. Настоящий +транспорт вынесен в `src/smtp.js` (как в своё время `human.js`), появился `test/mailer.test.js`. + +### Р-х4 — тест-пустышка про рабочую папку ✅ + +Написан настоящий: запуск **из чужого каталога** и **без** явной рабочей папки. +**Проверено вырезанием:** подмена умолчания на `process.cwd()` → красное (прежний тест +оставался зелёным). + +### Р-х5 — расширение файла и имя по номеру баннера ✅ + +Сделано вместе с П8 (см. выше). + +### Р-х6 — `Number(env.X ?? '800')` ✅ + +Новый помощник `number(env, key, fallback)`: пустое значение — это «не задавал, возьми +обычное», а мусор — внятная ошибка вместо тихого `NaN`. Прежде пустой `HUMAN_DELAY_MS` +давал 0 (робот щёлкал с машинной скоростью — так антифрод и опознаёт бота), а нечисловой +`SMTP_PORT` — `NaN`, и письма переставали уходить без единого слова в журнале. + +## Зелёная отметка после всех хвостов + +- портал: **293/293, 1008 проверок, ~48 с**; +- робот: **57/57**; +- вызовов снятия заморозки — **четыре**, пятого не появилось. + ## Не начато -Хвосты денег Д1–Д6 (Д4 — вопрос владельца) и робота Р-х1–Р-х4, Р-х6 — лист v12 §7. -Мелочи — §8. +Мелочи — лист v12 §8. Задачи 16 и 17 — только с «go» владельца. Побочно закрыта половина **Р-х6**: ключ `REPORT_RETRY_DELAY_MS` добавлен сразу в правильной форме, но `HUMAN_DELAY_MS` и `SMTP_PORT` по-прежнему с той же миной (`??` ловит только