diff --git a/src/adclaw/campaigns/diagnostics.py b/src/adclaw/campaigns/diagnostics.py index 903345a..d44385c 100644 --- a/src/adclaw/campaigns/diagnostics.py +++ b/src/adclaw/campaigns/diagnostics.py @@ -366,8 +366,16 @@ async def run_smoke_test(meta, ad_account_id: str, actor: str) -> dict[str, Any] except Exception as exc: steps.append({"step": "create", "status": FAILED, "detail": str(exc)}) finally: - target = (synced.meta_campaign_id if synced else None) or created.get( - "campaign_id", + # `campaign` last, and it is the one that matters: sync_campaign + # assigns meta_campaign_id on the spec it was handed *before* it + # creates the ad set, so a mid-chain failure leaves the campaign + # live in the account while `synced` is still None and `created` is + # still empty. Without this fallback that campaign is abandoned — + # which is exactly what happened on the first real run. + target = ( + (synced.meta_campaign_id if synced else None) + or created.get("campaign_id") + or campaign.meta_campaign_id ) if target: try: diff --git a/tests/test_campaign_diagnostics.py b/tests/test_campaign_diagnostics.py index 181e0b5..8b79760 100644 --- a/tests/test_campaign_diagnostics.py +++ b/tests/test_campaign_diagnostics.py @@ -418,3 +418,49 @@ async def test_no_token_blocks_the_connection_test(monkeypatch): assert report["can_smoke_test"] is False assert report["smoke_test_blocking"] == ["Meta access token"] + + +# --- a half-built chain must not be abandoned --- + + +async def test_a_failure_after_the_campaign_exists_still_deletes_it(page): + """The real failure mode, reproduced. + + sync_campaign assigns meta_campaign_id on the spec before it creates + the ad set. When the ad set is rejected, `synced` is never bound and + `created` is never populated — so a cleanup keyed only on those two + leaves a live campaign sitting in the ad account. On the first real run + that is exactly what happened, and the reconciler then adopted the + orphan into the database. + """ + + class AdSetRejects(SmokeMeta): + async def create_ad_set(self, ad_account_id, **kwargs): + raise MetaError( + "Invalid parameter", + code=100, + subcode=2490487, + user_message="Bid amount or bid constraints required.", + ) + + meta = AdSetRejects() + + result = await run_smoke_test(meta, "act_1", actor="farman") + + assert result["passed"] is False + # The campaign was created, so it must have been deleted again. + assert meta.deleted == ["camp_1"] + cleanup = next(s for s in result["steps"] if s["step"] == "clean up") + assert cleanup["status"] == OK + assert "camp_1" in cleanup["detail"] + + +async def test_nothing_is_deleted_when_the_campaign_was_never_created(page): + """No campaign id means no cleanup call — not a delete of None.""" + meta = SmokeMeta() + meta.fail_on = "campaign" + + result = await run_smoke_test(meta, "act_1", actor="farman") + + assert meta.deleted == [] + assert not [s for s in result["steps"] if s["step"] == "clean up"]