chore(#3197): cian-login — без холостой аренды и ложного отказа (/login сайдкара override не берёт); честные докстринги

Code-review хвоста #3197: на `cian-login` аренда из пула не доходила до сайдкара.
`BrowserFetcher._post_login` не кладёт `payload["proxy"]` (это делают только
`fetch`/`fetch_json`), а на приёме `login_handler` (browser/server.py:2814-2817)
зовёт `_no_live_proxy(provider, None)` и `_ensure_browser(provider)` без override —
`/login` proxy-override не принимает вовсе. Lease брался в `__aenter__` и
освобождался в `__aexit__` без пользы и без health-вердикта по узлу.

Хуже холостого хода: при пустом пуле в production `_acquire_lease` поднимает
`NoProxyAvailableError` ДО POST, `cian_auto_login` ловит любое `Exception` →
`502 Browser login failed`. То есть единственная ручка ВОССТАНОВЛЕНИЯ cian-сессии
отказывала ровно во время инцидента с пулом. Комментарий в admin.py при этом
утверждал, что фикс закрывает InvalidIP на логине — неправда.

Убран `proxy_provider=` (фабрика остаётся ради endpoint/environment из одного
места). `use_pool` без провайдера фетчер игнорирует сам — `_acquire_lease`:
`use_pool AND provider is not None` — поэтому ни аренды, ни прод-отказа.
`domclick-detail-debug` не тронут: он ходит через `fetch`, где override реально
кладётся в тело и читается сайдкаром — там #3197 остаётся настоящим фиксом.

Тесты для cian обратные по значению и падают на HEAD ветки:
`test_cian_auto_login_does_not_lease_from_pool` (провайдер не передан, `acquire`
не вызван) и `test_cian_auto_login_survives_empty_pool_in_production` (пустой пул
на проде не отдаёт 502). Стаб cian — подкласс настоящего `BrowserFetcher`, чтобы
второе утверждение шло через реальный `_acquire_lease`, а не через заглушку.

Follow-up (отдельной задачей): сайдкар `/login` не принимает proxy override →
логин cian всегда с env-узла (сейчас выключенный узел 9).

tradein-mvp/backend/app/api/v1/admin.py

@@ -508,13 +508,16 @@ async def cian_auto_login(
         )
 
     try:
-        # #3197 (хвост): через фабрику, а не BrowserFetcher(source=, endpoint=) напрямую —
-        # прямая конструкция не проставляла proxy_provider/use_pool/environment, и сайдкар
-        # логинился со своего env-узла SCRAPER_PROXY_URL мимо пула (на проде это выключенный
-        # узел 9: 407 → camoufox InvalidIP), а прод-отказ «пул пуст» (#2616) тут был мёртв.
-        async with build_browser_fetcher(
-            RealScraperConfig(), "cian", proxy_provider=_kit_proxy_provider()
-        ) as fetcher:
+        # #3197 (хвост): через фабрику (endpoint/environment из одного места), но
+        # НАМЕРЕННО без proxy_provider. `/login` сайдкара proxy-override не принимает
+        # (browser/server.py:2814-2817 — `_no_live_proxy(provider, None)`; и сам
+        # `_post_login` не кладёт payload["proxy"], это делают только fetch/fetch_json) —
+        # логин идёт с env-узла сайдкара. Аренда здесь была бы холостой и при пустом пуле
+        # блокировала бы ручку восстановления (`_acquire_lease` → NoProxyAvailableError →
+        # 502 ровно во время инцидента с пулом). Пул для логина — отдельная задача сайдкара.
+        # `use_pool` без провайдера фетчер игнорирует (`_acquire_lease`: use_pool AND
+        # provider is not None), поэтому передавать его тут безвредно, но и бесполезно.
+        async with build_browser_fetcher(RealScraperConfig(), "cian") as fetcher:
             raw_cookies = await fetcher.login(
                 url=settings.cian_login_url,
                 email=email,

tradein-mvp/backend/tests/test_3197_admin_debug_browser_pool_wiring.py

@@ -1,19 +1,34 @@
-"""#3197 (хвост) — две служебные ручки admin.py строили BrowserFetcher мимо пула.
+"""#3197 (хвост) — две служебные ручки admin.py и пул прокси: одна чинится, вторая НЕТ.
 
-`POST /admin/scrape/cian/auto-login` и `POST /admin/scrape/domclick/debug/detail-fetch`
-конструировали `BrowserFetcher(source=..., endpoint=settings.browser_http_endpoint)`
-напрямую — без `proxy_provider`/`use_pool`/`environment`, единственных трёх аргументов,
-которые кладут "proxy" в тело POST /fetch сайдкара. Без них сайдкар берёт свой env-узел
-(`SCRAPER_PROXY_URL`), на проде это выключенный узел 9 (407 → camoufox `InvalidIP`), а
-прод-отказ «пул пуст → не ходить на env/direct» (#2616) на этих путях был мёртв: он
-смотрит на `environment`, который до конструктора не доезжал. Соседние точки того же
-эпика уже переведены: yandex newbuilding (#3389), cian history (#3197), domclick/avito.
+`POST /admin/scrape/domclick/debug/detail-fetch` — **настоящий фикс**. Он ходит через
+`BrowserFetcher.fetch`, а `fetch`/`fetch_json` — единственные методы, которые кладут
+`payload["proxy"]` в тело POST /fetch сайдкара, и сайдкар этот override читает
+(`_resolve_proxy_override` → `_ensure_browser(provider, proxy_override=...)`). Прямая
+конструкция `BrowserFetcher(source=, endpoint=)` не проставляла
+`proxy_provider`/`use_pool`/`environment` — без них сайдкар брал свой env-узел
+(`SCRAPER_PROXY_URL`, на проде это выключенный узел 9: 407 → camoufox `InvalidIP`), а
+прод-отказ «пул пуст → не ходить на мёртвый env» (#2616) тут был мёртв: он смотрит на
+`environment`, который до конструктора не доезжал. Тест меряет ЗНАЧЕНИЯ kwargs.
 
-Тест меряет ЗНАЧЕНИЯ kwargs, а не то, из какого модуля взят класс: подделка ставится и на
-`scraper_kit.providers._base.BrowserFetcher` (путь через фабрику), и на
-`app.api.v1.admin.BrowserFetcher` (прямая конструкция, как было до правки, `create=True` —
-после правки такого имени в модуле нет). Поэтому на откате красное читается как «в
-конструктор не передан провайдер», а не как «мок не сработал».
+`POST /admin/scrape/cian/auto-login` — **намеренно без пула**, и это проверяется обратными
+по значению утверждениями. Логин идёт не через `fetch`, а через `login` → `_post_login`,
+который `payload["proxy"]` не кладёт вовсе; на приёме `login_handler`
+(`browser/server.py:2814-2817`) зовёт `_no_live_proxy(provider, None)` и
+`_ensure_browser(provider)` без override — то есть **сайдкар на `/login` proxy-override не
+принимает** и логинится с env-узла при любом теле запроса. Аренда на этом пути была бы
+холостой (взяли в `__aenter__`, отпустили в `__aexit__`, health-вердикта по узлу нет), а
+при пустом пуле в production `_acquire_lease` поднимает `NoProxyAvailableError` ДО POST —
+и единственная ручка ВОССТАНОВЛЕНИЯ сессии отдавала бы `502 Browser login failed` ровно во
+время инцидента с пулом. Поэтому здесь `proxy_provider` не передаётся (фабрика остаётся
+ради endpoint/environment из одного места); пул для логина — отдельная задача сайдкара.
+
+Подделка ставится и на `scraper_kit.providers._base.BrowserFetcher` (путь через фабрику), и
+на `app.api.v1.admin.BrowserFetcher` (прямая конструкция, как было до #3197, `create=True` —
+сейчас такого имени в модуле нет). Дубль нужен, чтобы файл был запускаем и против прежних
+состояний исходников: красное тогда читается как «значение не то», а не «мок не сработал».
+Стаб cian — ПОДКЛАСС настоящего `BrowserFetcher`: `__aenter__`/`_acquire_lease` исполняются
+по-настоящему (иначе тест на «нет ложного 502» проверял бы заглушку), застаблен только
+сетевой `login`.
 
 Сеть/БД/камуфокс замоканы; в сеть тест не ходит.
 """
@@ -29,6 +44,9 @@ from unittest.mock import AsyncMock, MagicMock, patch
 os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
 
 import pytest
+from fastapi import HTTPException
+from scraper_kit.browser_fetcher import BrowserFetcher
+from scraper_kit.contracts import ProxyLease
 
 from app.api.v1 import admin
 from app.services import cian_session as cian_session_svc
@@ -54,6 +72,45 @@ class _CapturingFetcher:
         return {name: "v" for name in cian_session_svc.CIAN_REQUIRED_COOKIES}
 
 
+class _RealLeaseFetcher(BrowserFetcher):
+    """Настоящий `BrowserFetcher` (реальные `__aenter__`/`_acquire_lease`) без сети.
+
+    Наследование, а не заглушка: утверждение «пустой пул в production не роняет ручку»
+    обязано пройти через тот самый guard `_acquire_lease`, который поднимает
+    `NoProxyAvailableError`. Подменён только сетевой `login`.
+    """
+
+    captured: ClassVar[list[dict[str, Any]]] = []
+
+    def __init__(self, **kwargs: Any) -> None:
+        _RealLeaseFetcher.captured.append(kwargs)
+        super().__init__(**kwargs)
+
+    async def login(self, **_kwargs: Any) -> dict[str, str]:
+        return {name: "v" for name in cian_session_svc.CIAN_REQUIRED_COOKIES}
+
+
+class _SpyProvider:
+    """Пустой пул + спай: `acquire` пишет вызовы в ClassVar и всегда отдаёт None.
+
+    ClassVar, а не поле инстанса: `_kit_proxy_provider()` конструирует провайдер сам,
+    и «ни разу не позвали» должно покрывать в том числе «даже не создали».
+    """
+
+    acquired: ClassVar[list[str]] = []
+    released: ClassVar[list[int]] = []
+
+    def acquire(self, provider: str) -> ProxyLease | None:
+        _SpyProvider.acquired.append(provider)
+        return None
+
+    def release(self, lease: ProxyLease) -> None:
+        _SpyProvider.released.append(lease.id)
+
+    def mark_health(self, lease: ProxyLease, *, ok: bool, error: str | None = None) -> None:
+        return None
+
+
 @pytest.fixture
 def _pool_on(monkeypatch: pytest.MonkeyPatch) -> None:
     """Curl-флаг включён всегда: это он открывает `_kit_proxy_provider()` (гейт #2163).
@@ -64,51 +121,81 @@ def _pool_on(monkeypatch: pytest.MonkeyPatch) -> None:
     from app.core.config import settings
 
     monkeypatch.setattr(settings, "use_proxy_pool_curl", True)
+    monkeypatch.setattr(admin, "RealProxyProvider", _SpyProvider)
     _CapturingFetcher.captured = []
+    _RealLeaseFetcher.captured = []
+    _SpyProvider.acquired = []
+    _SpyProvider.released = []
 
 
-def _assert_wiring(source: str, *, use_pool: bool, environment: str) -> None:
-    assert len(_CapturingFetcher.captured) == 1, "ручка обязана построить ровно один фетчер"
-    kwargs = _CapturingFetcher.captured[0]
-    assert kwargs["source"] == source
+def _cian_login_settings(monkeypatch: pytest.MonkeyPatch, *, use_pool: bool, env: str) -> None:
+    from app.core.config import settings
+
+    monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool)
+    monkeypatch.setattr(settings, "environment", env)
+    monkeypatch.setattr(settings, "cookie_encryption_key", "k" * 32)
+    monkeypatch.setattr(settings, "cian_login_email", "a@b.c")
+    monkeypatch.setattr(settings, "cian_login_password", "pw")
+
+
+def _patched_cian_login(stack: ExitStack, fetcher_cls: type) -> None:
+    for target in _TARGETS:
+        stack.enter_context(patch(target, fetcher_cls, create=True))
+    stack.enter_context(
+        patch.object(
+            cian_session_svc, "verify_session", AsyncMock(return_value={"user": {"userId": 7}})
+        )
+    )
+    stack.enter_context(patch.object(cian_session_svc, "save_session", MagicMock()))
+
+
+@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")])
+@pytest.mark.usefixtures("_pool_on")
+async def test_cian_auto_login_does_not_lease_from_pool(
+    monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str
+) -> None:
+    """Логин пул НЕ арендует: сайдкар на `/login` proxy-override не берёт (см. докстринг)."""
+    _cian_login_settings(monkeypatch, use_pool=use_pool, env=environment)
+
+    with ExitStack() as stack:
+        _patched_cian_login(stack, _RealLeaseFetcher)
+        result = await admin.cian_auto_login(db=MagicMock(), body=None)
+
+    assert result["ok"] is True and result["userId"] == 7
+    assert len(_RealLeaseFetcher.captured) == 1, "ручка обязана построить ровно один фетчер"
+    kwargs = _RealLeaseFetcher.captured[0]
     # .get(), а не [] — красное должно читаться как «значение не то», а не KeyError.
-    assert kwargs.get("proxy_provider") is not None, "без провайдера пул не подключится"
-    assert kwargs.get("use_pool") is use_pool, "флаг пула должен доезжать из конфига"
-    # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв.
-    assert kwargs.get("environment") == environment
-    # endpoint не должен потеряться при переезде на фабрику (#2322: без него TypeError).
+    assert kwargs.get("proxy_provider") is None, "аренда на /login холостая — провайдер не нужен"
+    assert _SpyProvider.acquired == [], "lease взят впустую (сайдкар его всё равно не увидит)"
+    # endpoint из фабрики не должен потеряться (#2322: без него TypeError).
     from app.core.config import settings
 
     assert kwargs.get("endpoint") == settings.browser_http_endpoint
 
 
-@pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")])
 @pytest.mark.usefixtures("_pool_on")
-async def test_cian_auto_login_wires_proxy_pool(
-    monkeypatch: pytest.MonkeyPatch, use_pool: bool, environment: str
+async def test_cian_auto_login_survives_empty_pool_in_production(
+    monkeypatch: pytest.MonkeyPatch,
 ) -> None:
-    """Логин-браузер ходит через узел пула, а не через env-прокси сайдкара."""
-    from app.core.config import settings
+    """Пустой пул на проде НЕ ломает ручку восстановления сессии (нет ложного 502).
 
-    monkeypatch.setattr(settings, "use_proxy_pool_browser", use_pool)
-    monkeypatch.setattr(settings, "environment", environment)
-    monkeypatch.setattr(settings, "cookie_encryption_key", "k" * 32)
-    monkeypatch.setattr(settings, "cian_login_email", "a@b.c")
-    monkeypatch.setattr(settings, "cian_login_password", "pw")
+    Ровно тот сценарий, ради которого пул отсюда убран: `_acquire_lease` при
+    `use_pool + provider + production + пустой пул` поднимает `NoProxyAvailableError`,
+    `cian_auto_login` ловит любое `Exception` и отдаёт `502 Browser login failed` — то
+    есть инцидент с пулом закрывал бы единственный способ переполучить cian-сессию,
+    хотя логину пул не нужен (сайдкар proxy-override на `/login` не принимает).
+    """
+    _cian_login_settings(monkeypatch, use_pool=True, env="production")
 
     with ExitStack() as stack:
-        for target in _TARGETS:
-            stack.enter_context(patch(target, _CapturingFetcher, create=True))
-        stack.enter_context(
-            patch.object(
-                cian_session_svc, "verify_session", AsyncMock(return_value={"user": {"userId": 7}})
-            )
-        )
-        stack.enter_context(patch.object(cian_session_svc, "save_session", MagicMock()))
-        result = await admin.cian_auto_login(db=MagicMock(), body=None)
+        _patched_cian_login(stack, _RealLeaseFetcher)
+        try:
+            result = await admin.cian_auto_login(db=MagicMock(), body=None)
+        except HTTPException as exc:
+            # pytest.fail, а не re-raise: красное должно называть статус и detail.
+            pytest.fail(f"пустой пул уронил ручку восстановления: {exc.status_code} {exc.detail}")
 
     assert result["ok"] is True and result["userId"] == 7
-    _assert_wiring("cian", use_pool=use_pool, environment=environment)
 
 
 @pytest.mark.parametrize(("use_pool", "environment"), [(True, "production"), (False, "dev")])
@@ -147,4 +234,15 @@ async def test_domclick_debug_detail_wires_proxy_pool(
         result = await admin.debug_domclick_detail_fetch(body=body, db=MagicMock())
 
     assert result.ok is True
-    _assert_wiring("domclick", use_pool=use_pool, environment=environment)
+    assert len(_CapturingFetcher.captured) == 1, "ручка обязана построить ровно один фетчер"
+    kwargs = _CapturingFetcher.captured[0]
+    assert kwargs["source"] == "domclick"
+    # .get(), а не [] — красное должно читаться как «значение не то», а не KeyError.
+    assert kwargs.get("proxy_provider") is not None, "без провайдера пул не подключится"
+    assert kwargs.get("use_pool") is use_pool, "флаг пула должен доезжать из конфига"
+    # #2616 шаг 1: без environment отказ «пул пуст» на этом пути мёртв.
+    assert kwargs.get("environment") == environment
+    # endpoint не должен потеряться при переезде на фабрику (#2322: без него TypeError).
+    from app.core.config import settings
+
+    assert kwargs.get("endpoint") == settings.browser_http_endpoint

tradein-mvp/packages/scraper-kit/src/scraper_kit/providers/_base.py

@@ -212,8 +212,13 @@ def build_browser_fetcher(
     Служебные ручки `app/api/v1/admin.py` (cian-login, domclick-detail-debug) были
     последними прямыми конструкциями мимо фабрики — переведены сюда же (#3197 хвост);
     прямые конструкции в `orchestration/pipeline.py` и в backfill-задачах пул получают
-    своими kwargs. `use_pool` при `proxy_provider is None` эффективно игнорируется
-    `BrowserFetcher` (env-fallback, см. `browser_fetcher.py::_pool_proxy`).
+    своими kwargs. Живой `proxy_provider=None` остался ровно один — cian-login: сайдкар
+    на `/login` proxy-override не берёт (`browser/server.py::login_handler` →
+    `_no_live_proxy(provider, None)`, а `_post_login` не кладёт `payload["proxy"]`), так
+    что аренда там была бы холостой, а на проде при пустом пуле роняла бы ручку
+    восстановления в 502. `use_pool` при `proxy_provider is None` игнорируется
+    `BrowserFetcher` (`_acquire_lease`: `use_pool AND provider is not None`) — ни аренды,
+    ни прод-отказа, поведение как до фабрики.
 
     `fetch_timeout_s=None` (дефолт) → используется дефолт `BrowserFetcher`
     (120s). Явный таймаут передаёт ровно один call-site — `yandex/serp.py` (30s);