fix: require strict fetch for title cleanup (#1206)

## ℹ️ Description
- Link to the related issue(s): Issue #
- Fixes a remaining fail-closed gap in BEFORE_PUBLISH title cleanup for
ID-less ads when the published-ad list cannot be fetched completely.

## 📋 Changes Summary
- Require an additional strict published-ad fetch when BEFORE_PUBLISH
title cleanup may match by title for ID-less ads.
- Fail closed for ID-less publish candidates if strict published-ad
fetch is incomplete, before submit/delete actions.
- Keep ID-based candidates on the normal published-ad list so they can
continue safely.
- Add regression tests for strict-fetch usage and fail-closed behavior.
- Add German translations for the new log messages.

### ⚙️ Type of Change
Select the type(s) of change(s) included in this pull request:
- [x] 🐞 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (adds new functionality without breaking existing
usage)
- [ ] 💥 Breaking change (changes that might break existing user setups,
scripts, or configurations)

## ✅ Checklist
Before requesting a review, confirm the following:
- [x] I have reviewed my changes to ensure they meet the project's
standards.
- [x] I have tested my changes and ensured that all tests pass (`pdm run
test`).
- [x] I have formatted the code (`pdm run format`).
- [x] I have verified that linting passes (`pdm run lint`).
- [x] I have updated documentation where necessary.

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

Validation performed locally:
- `pdm run pytest tests/unit/test_publishing_workflow.py
tests/unit/test_delete_flow.py`
- `pdm run format`
- `pdm run lint`
- `pdm run test`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Enhanced publishing workflow to use more precise “published ad” data
when title-based cleanup is enabled, improving match-and-replace
behavior.
* **Bug Fixes**
* Publishing now fails safely when strict published-ad data is
incomplete, avoiding retries and deletion actions when precision can’t
be guaranteed.
* For ads lacking a known identifier, the workflow intelligently selects
the best available published-ad list or skips with a warning when strict
data is unavailable.
* **Tests**
* Added unit test coverage for strict-vs-non-strict published-ad
fetching scenarios and fail-closed behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

src/kleinanzeigen_bot/publishing_workflow.py

@@ -29,7 +29,7 @@ from . import publishing_persistence as _publishing_persistence
 from . import publishing_submission as _publishing_submission
 from .model.ad_model import Ad, AdUpdateStrategy
 from .model.config_model import Config
-from .published_ads import PublishedAd, ad_matches_id
+from .published_ads import PublishedAd, PublishedAdsFetchIncompleteError, ad_matches_id
 from .utils import loggers as _loggers
 from .utils.exceptions import CategoryResolutionError, PublishSubmissionUncertainError
 from .utils.i18n import pluralize
@@ -183,6 +183,41 @@ async def publish_ad(
         raise PostPublishPersistenceError(ad_id = ad_id, ad_title = ad_cfg.title, original = ex) from ex
 
 
+async def _fetch_published_ads_for_publish(
+    web:WebScrapingMixin,
+    root_url:str,
+    config:Config,
+    ad_cfgs:list[tuple[str, Ad, dict[str, Any]]],
+    *,
+    keep_old_ads:bool,
+) -> tuple[list[PublishedAd], list[PublishedAd] | None, bool]:
+    """Fetch published ads for publish flow, strictly when title cleanup needs it."""
+    require_strict_fetch = (
+        not keep_old_ads
+        and config.publishing.delete_old_ads == "BEFORE_PUBLISH"
+        and config.publishing.delete_old_ads_by_title
+        and any(ad_cfg.id is None for _ad_file, ad_cfg, _ad_cfg_orig in ad_cfgs)
+    )
+    published_ads_list = await published_ads.fetch_published_ads(web, root_url)
+    strict_published_ads_list:list[PublishedAd] | None = None
+
+    if require_strict_fetch:
+        try:
+            strict_published_ads_list = await published_ads.fetch_published_ads(
+                web,
+                root_url,
+                strict = True,
+            )
+        except PublishedAdsFetchIncompleteError as ex:
+            LOG.error(
+                "Skipping title-based publishes because full published-ad list could not "
+                "be fetched before publish: %s",
+                ex,
+            )
+
+    return published_ads_list, strict_published_ads_list, require_strict_fetch
+
+
 async def publish_ads(
     web:WebScrapingMixin,
     ad_cfgs:list[tuple[str, Ad, dict[str, Any]]],
@@ -210,13 +245,34 @@ async def publish_ads(
     count = 0
     failed_count = 0
     max_retries = SUBMISSION_MAX_RETRIES
-
-    published_ads_list = await published_ads.fetch_published_ads(web, root_url)
+    published_ads_list, strict_published_ads_list, require_strict_fetch = await _fetch_published_ads_for_publish(
+        web,
+        root_url,
+        config,
+        ad_cfgs,
+        keep_old_ads = keep_old_ads,
+    )
 
     for idx, (ad_file, ad_cfg, ad_cfg_orig) in enumerate(ad_cfgs, start = 1):
         LOG.info("Processing %s/%s: '%s' from [%s]...", idx, len(ad_cfgs), ad_cfg.title, ad_file)
 
-        if any(ad_matches_id(x, ad_cfg.id) and x.get("state") == "paused" for x in published_ads_list):
+        published_ads_for_matching = (
+            strict_published_ads_list
+            if ad_cfg.id is None and strict_published_ads_list is not None
+            else published_ads_list
+        )
+
+        if ad_cfg.id is None and strict_published_ads_list is None and require_strict_fetch:
+            LOG.warning(
+                "Skipping '%s' because strict published-ad fetch failed before publish. "
+                "Retry by re-running the publish after transient API issues have resolved.",
+                ad_cfg.title,
+            )
+            count += 1
+            failed_count += 1
+            continue
+
+        if any(ad_matches_id(x, ad_cfg.id) and x.get("state") == "paused" for x in published_ads_for_matching):
             LOG.info("Skipping because ad is reserved")
             continue
 
@@ -233,7 +289,7 @@ async def publish_ads(
                 ad_cfg.price_reduction_count = baseline_price_reduction_count
                 await publish_ad(
                     web, ad_file, ad_cfg, ad_cfg_orig,
-                    published_ads_list, AdUpdateStrategy.REPLACE,
+                    published_ads_for_matching, AdUpdateStrategy.REPLACE,
                     root_url = root_url, config = config,
                     keep_old_ads = keep_old_ads,
                     config_file_path = config_file_path,
@@ -313,7 +369,7 @@ async def publish_ads(
                 )
 
             await delete_old_ad_if_needed(
-                web, ad_cfg, published_ads_list,
+                web, ad_cfg, published_ads_for_matching,
                 timing = "AFTER_PUBLISH",
                 keep_old_ads = keep_old_ads,
                 config = config,

src/kleinanzeigen_bot/resources/translations.de.yaml

@@ -185,9 +185,14 @@ kleinanzeigen_bot/publishing_workflow.py:
     " -> effective ad meta:": " -> effektive Anzeigen-Metadaten:"
     "Post-publish persistence failed for '%s' (ad ID %s - ad is live on Kleinanzeigen but local YAML may be out of sync)": "Persistenz nach Veröffentlichung fehlgeschlagen für '%s' (Anzeigen-ID %s - Anzeige ist auf Kleinanzeigen live, aber die lokale YAML ist möglicherweise nicht synchron)"
 
+  _fetch_published_ads_for_publish:
+    "Skipping title-based publishes because full published-ad list could not be fetched before publish: %s": "Titelbasierte Veröffentlichungen werden übersprungen, weil die vollständige Liste veröffentlichter Anzeigen vor der Veröffentlichung nicht abgerufen werden konnte: %s"
+
   publish_ads:
     "Processing %s/%s: '%s' from [%s]...": "Verarbeite %s/%s: '%s' von [%s]..."
     "Skipping because ad is reserved": "Überspringen, da Anzeige reserviert ist"
+    ? "Skipping '%s' because strict published-ad fetch failed before publish. Retry by re-running the publish after transient API issues have resolved."
+    : "'%s' wird übersprungen, weil der strikte Abruf veröffentlichter Anzeigen vor der Veröffentlichung fehlgeschlagen ist. Wiederholen Sie die Veröffentlichung, nachdem vorübergehende API-Probleme behoben sind."
     " -> Could not confirm publishing for '%s', but ad may be online": " -> Veröffentlichung für '%s' konnte nicht bestätigt werden, aber Anzeige ist möglicherweise online"
     "Attempt %s/%s failed for '%s': %s. Retrying...": "Versuch %s/%s fehlgeschlagen für '%s': %s. Erneuter Versuch..."
     "Attempt %s/%s for '%s' reached submit boundary but failed: %s. Not retrying to prevent duplicate listings.": "Versuch %s/%s für '%s' hat die Submit-Grenze erreicht, ist aber fehlgeschlagen: %s. Kein erneuter Versuch, um doppelte Anzeigen zu vermeiden."

tests/unit/test_publishing_workflow.py

@@ -13,7 +13,7 @@ from collections.abc import Iterator
 from contextlib import ExitStack, contextmanager
 from pathlib import Path, PureWindowsPath
 from typing import Any
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, call, patch
 
 import pytest
 from nodriver.core.connection import ProtocolException
@@ -24,6 +24,7 @@ from kleinanzeigen_bot.model.config_model import (
     AutoPriceReductionConfig,
     DiagnosticsConfig,
 )
+from kleinanzeigen_bot.published_ads import PublishedAdsFetchIncompleteError
 from kleinanzeigen_bot.publishing_workflow import SUBMISSION_MAX_RETRIES, PostPublishPersistenceError
 from kleinanzeigen_bot.utils.exceptions import CategoryResolutionError, PublishSubmissionUncertainError
 from tests.conftest import build_published_ads, build_update_ad
@@ -401,6 +402,117 @@ class TestKleinanzeigenBotPublishAdsBasics:
             assert any("Persistence failed for 'Test Title' after ad submission" in record.getMessage() for record in caplog.records)
             assert any("Ad ID: 12345" in record.getMessage() for record in caplog.records)
 
+    @pytest.mark.asyncio
+    async def test_publish_ads_fetches_published_ads_strictly_for_id_less_title_cleanup(
+        self,
+        test_bot:KleinanzeigenBot,
+        base_ad_config:dict[str, Any],
+        caplog:pytest.LogCaptureFixture,
+    ) -> None:
+        test_bot.config.publishing.delete_old_ads = "BEFORE_PUBLISH"
+        test_bot.config.publishing.delete_old_ads_by_title = True
+
+        ad_cfg = Ad.model_validate(base_ad_config)
+        ad_cfg_orig = copy.deepcopy(base_ad_config)
+        ad_file = "ad.yaml"
+
+        non_strict_ads = [{"id": 10, "state": "active"}]
+        strict_ads = [
+            {"id": 10, "state": "active"},
+            {"id": 11, "state": "active"},
+        ]
+
+        with (
+            patch(
+                "kleinanzeigen_bot.published_ads.fetch_published_ads",
+                new_callable = AsyncMock,
+                side_effect = [non_strict_ads, strict_ads],
+            ) as fetch_mock,
+            patch("kleinanzeigen_bot.publishing_workflow.publish_ad", new_callable = AsyncMock) as publish_mock,
+            patch.object(test_bot, "web_await", new_callable = AsyncMock, return_value = True),
+            patch("kleinanzeigen_bot.delete_flow.delete_ad", new_callable = AsyncMock),
+        ):
+            await test_bot.publish_ads([(ad_file, ad_cfg, ad_cfg_orig)])
+
+            fetch_mock.assert_has_awaits([
+                call(test_bot, test_bot.root_url),
+                call(test_bot, test_bot.root_url, strict = True),
+            ])
+            assert publish_mock.await_count == 1
+            assert publish_mock.call_args.args[4] == strict_ads
+
+            summary = [record for record in caplog.records if "DONE:" in record.getMessage()]
+            assert any("DONE: (Re-)published 1" in record.getMessage() for record in summary)
+
+    @pytest.mark.asyncio
+    async def test_publish_ads_fails_closed_when_strict_published_ads_fetch_fails_for_id_less_candidates(
+        self,
+        test_bot:KleinanzeigenBot,
+        base_ad_config:dict[str, Any],
+        caplog:pytest.LogCaptureFixture,
+    ) -> None:
+        test_bot.config.publishing.delete_old_ads = "BEFORE_PUBLISH"
+        test_bot.config.publishing.delete_old_ads_by_title = True
+
+        ad_cfg = Ad.model_validate(base_ad_config)
+        ad_cfg_orig = copy.deepcopy(base_ad_config)
+        ad_file = "ad.yaml"
+
+        with (
+            patch(
+                "kleinanzeigen_bot.published_ads.fetch_published_ads",
+                new_callable = AsyncMock,
+                side_effect = [[], PublishedAdsFetchIncompleteError("incomplete published-ad fetch")],
+            ) as fetch_mock,
+            patch("kleinanzeigen_bot.publishing_workflow.publish_ad", new_callable = AsyncMock) as publish_mock,
+            patch.object(test_bot, "web_sleep", new_callable = AsyncMock) as sleep_mock,
+            patch.object(test_bot, "web_await", new_callable = AsyncMock) as await_mock,
+            patch("kleinanzeigen_bot.delete_flow.delete_ad", new_callable = AsyncMock) as delete_mock,
+            caplog.at_level(logging.INFO),
+        ):
+            await test_bot.publish_ads([(ad_file, ad_cfg, ad_cfg_orig)])
+
+            assert fetch_mock.await_count == 2
+            publish_mock.assert_not_awaited()
+            sleep_mock.assert_not_awaited()
+            await_mock.assert_not_awaited()
+            delete_mock.assert_not_awaited()
+
+            summary = [record for record in caplog.records if "DONE:" in record.getMessage()]
+            assert any("DONE: (Re-)published 0 ads (1 failed after retries)" in record.getMessage() for record in summary)
+
+    @pytest.mark.asyncio
+    async def test_publish_ads_keep_old_does_not_require_strict_title_cleanup_fetch(
+        self,
+        test_bot:KleinanzeigenBot,
+        base_ad_config:dict[str, Any],
+    ) -> None:
+        test_bot.config.publishing.delete_old_ads = "BEFORE_PUBLISH"
+        test_bot.config.publishing.delete_old_ads_by_title = True
+        test_bot.keep_old_ads = True
+
+        ad_cfg = Ad.model_validate(base_ad_config)
+        ad_cfg_orig = copy.deepcopy(base_ad_config)
+        ad_file = "ad.yaml"
+        published_ads = [{"id": 10, "state": "active"}]
+
+        with (
+            patch(
+                "kleinanzeigen_bot.published_ads.fetch_published_ads",
+                new_callable = AsyncMock,
+                return_value = published_ads,
+            ) as fetch_mock,
+            patch("kleinanzeigen_bot.publishing_workflow.publish_ad", new_callable = AsyncMock) as publish_mock,
+            patch.object(test_bot, "web_await", new_callable = AsyncMock, return_value = True),
+            patch("kleinanzeigen_bot.delete_flow.delete_ad", new_callable = AsyncMock) as delete_mock,
+        ):
+            await test_bot.publish_ads([(ad_file, ad_cfg, ad_cfg_orig)])
+
+            fetch_mock.assert_awaited_once_with(test_bot, test_bot.root_url)
+            publish_mock.assert_awaited_once()
+            assert publish_mock.call_args.args[4] == published_ads
+            delete_mock.assert_not_awaited()
+
     @pytest.mark.asyncio
     async def test_update_ads_persistence_failure_is_not_retried_and_still_processes_next_ad(
         self,