diff --git a/src/roomieorder/cli.py b/src/roomieorder/cli.py index f2d1ac7..2a1fb51 100644 --- a/src/roomieorder/cli.py +++ b/src/roomieorder/cli.py @@ -39,7 +39,7 @@ from roomieorder.notify import build_notifier from roomieorder.retention import prune_shots from roomieorder.sheets import build_sheets -from roomieorder.store import Store +from roomieorder.store import Store, is_login_pause def _setup_logging(verbose: bool) -> None: @@ -227,6 +227,7 @@ def wait_for_operator(page: object) -> None: # the persistent ("remember me") cookies were written. if purchaser.verify_session(): # type: ignore[attr-defined] click.echo("✓ signed in — session persisted (survives restarts and the worker)") + _clear_login_pause(config, provider) else: click.echo( "⚠️ the saved profile reloads signed OUT — the session didn’t persist. " @@ -235,6 +236,27 @@ def wait_for_operator(page: object) -> None: ) +def _clear_login_pause(config: Config, provider: str) -> None: + """After a confirmed hand-login, resume the worker if it was paused by *this* + store's sign-in wall. + + The sign-in-wall ``pause_reason`` literally instructs the operator to run + ``roomieorder login``, yet nothing else resets the flag — so without this the + operator logs in, is told the session persisted, taps the button, and gets the + same "logged out" message back: an inescapable loop. Only a matching + login/session pause is cleared (``store.is_login_pause``); a challenge, + needs-review, crash or manual pause stays operator-gated. Resolves the same + ``config.db_path`` the co-located service uses, so it clears the live flag.""" + store = Store(config.db_path) + store.init_db() + try: + if store.is_paused() and is_login_pause(store.pause_reason(), provider): + store.set_paused(False) + click.echo(f"✓ worker was paused for {provider} login — resumed") + finally: + store.close() + + @main.command(name="dry-run") @click.argument("item_key") @_PROVIDER_OPT @@ -528,6 +550,17 @@ def doctor(check_login: bool) -> None: def line(state: str, label: str, detail: str) -> None: click.echo(f"{state:4} {label:14} {detail}") + # ── which universe is this? ── + # ROOMIEORDER_DB / _PROFILE_DIR / _CATALOG default to CWD-relative paths, so a + # doctor run from the repo checkout can read a *different* DB and profile than + # the live systemd service (which cd's to its state dir first). Print the + # resolved absolutes + CWD so that mismatch is self-evident and a stale repo + # jar is never mistaken for the live one. + line("ok", "cwd", os.getcwd()) + line("ok", "paths/db", str(config.db_path.resolve())) + line("ok", "paths/profile", str(config.profile_dir.resolve())) + line("ok", "paths/catalog", str(config.catalog_path.resolve())) + # ── config / safety ── line("ok", "dry_run", str(config.dry_run)) line("ok", "daily_cap", f"${config.daily_cap:.2f}") @@ -603,7 +636,7 @@ def line(state: str, label: str, detail: str) -> None: if path.exists(): stamp = datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc).isoformat() present = "present" if check_login else "present (login unverified — run dump-dom)" - line("ok", f"profile/{label}", f"{present}, mtime {stamp}") + line("ok", f"profile/{label}", f"{present}, {path.resolve()}, mtime {stamp}") else: line("warn", f"profile/{label}", f"missing {path} — run `roomieorder login --provider {label}`") continue diff --git a/src/roomieorder/main.py b/src/roomieorder/main.py index 0211038..eef1fc4 100644 --- a/src/roomieorder/main.py +++ b/src/roomieorder/main.py @@ -35,7 +35,7 @@ from roomieorder.purchase import PurchaseResult, build_purchaser from roomieorder.retention import prune_shots from roomieorder.sheets import SheetsClient, build_sheets -from roomieorder.store import QueueRow, Store +from roomieorder.store import QueueRow, Store, is_login_pause _logger = logging.getLogger(__name__) @@ -270,6 +270,32 @@ def _session_check_tick(self, *, now: Optional[float] = None) -> None: f"`roomieorder login --provider {provider}` before the next order" ) _logger.warning("%s session probe: logged OUT", provider) + continue + # Log the healthy outcome too (the negative branch alone left a silent + # probe with no journal trail — a live session was indistinguishable + # from a probe that never ran). Then self-heal: a transient sign-in + # wall can latch a pause that outlives the condition, so once the + # profile reloads signed in, clear a matching login pause. + _logger.info("%s session probe: logged in", provider) + self._resume_if_login_pause(provider) + + def _resume_if_login_pause(self, provider: str) -> None: + """Clear a login/session pause once ``provider`` is confirmed signed in. + + Deliberately narrow: only a pause raised by *that* store's sign-in wall + (``store.is_login_pause``) is auto-cleared — a captcha ``challenge``, a + ``needs_review`` restart pause, a worker crash, a spend cap or a manual CLI + pause all stay latched for the operator. Auto-resume can re-arm real + ordering when ``DRY_RUN`` is off, so the gate is intentionally strict.""" + if not self.store.is_paused(): + return + if not is_login_pause(self.store.pause_reason(), provider): + return + self.store.set_paused(False) + self.notifier.send( + f"✅ {provider.capitalize()} session re-established — worker resumed" + ) + _logger.info("worker resumed: %s session re-established", provider) # ─────────── per-row processing ─────────── diff --git a/src/roomieorder/purchase.py b/src/roomieorder/purchase.py index 09a6dc7..4a55b54 100644 --- a/src/roomieorder/purchase.py +++ b/src/roomieorder/purchase.py @@ -53,7 +53,7 @@ from roomieorder.config import Config from roomieorder.guards import GuardResult from roomieorder.logutil import correlated -from roomieorder.store import Status +from roomieorder.store import LOGIN_PAUSE_MARKER, Status # Each purchaser drives exactly one store's source shape; bind it so the buy # skeleton on the base can pass the concrete CostcoSource/AmazonSource through to @@ -1556,11 +1556,15 @@ def _signin_required(self, page: "Page", item_key: str, where: str) -> PurchaseR it's logged out. Pause with a clear next step rather than mislabeling the login form as a review screenshot.""" shot = self._screenshot(page, item_key, f"signin_{where}") + # Build the remediation hint from LOGIN_PAUSE_MARKER (renders as + # "roomieorder login --provider ") so the pause_reason string this + # becomes always carries the marker that lets `login` / the session probe + # auto-clear it — see store.is_login_pause. Don't inline the command text. return PurchaseResult( status="challenge", message=( f"⚠️ {self.STORE_NAME} is logged out (hit the sign-in wall on the " - f"{where} page) — run `roomieorder login --provider {self.PROVIDER}`, then retry" + f"{where} page) — run `{LOGIN_PAUSE_MARKER} {self.PROVIDER}`, then retry" ), screenshot=shot, ) diff --git a/src/roomieorder/store.py b/src/roomieorder/store.py index 98e23b6..303ab1c 100644 --- a/src/roomieorder/store.py +++ b/src/roomieorder/store.py @@ -52,6 +52,31 @@ # hand it to the operator instead (see ``recover_stale`` and ``claim_next_pending``). MAX_ATTEMPTS = 1 +# Marker embedded in a worker-pause reason raised because a store profile got +# bounced to the sign-in wall (``purchase.BasePurchaser._signin_required``). It is +# the literal remediation command in that message, and the pause_reason string is +# the *only* thing a pause carries in ``worker_state`` — so it is also how an +# automated login/session recovery recognises a pause it is allowed to clear. A +# captcha ``challenge``, a ``needs_review`` restart pause, a worker crash, a spend +# cap and a manual CLI pause all lack this marker and stay operator-gated. +# ``_signin_required`` builds its message around this constant; keep them in +# lockstep so a re-worded message can't silently break auto-resume. +LOGIN_PAUSE_MARKER = "roomieorder login --provider" + + +def is_login_pause(reason: str, provider: Optional[str] = None) -> bool: + """True when ``reason`` is a logged-out sign-in-wall pause — the only pause an + automated login/session recovery may clear. + + Pass ``provider`` to additionally require the pause be about *that* store, so a + successful Amazon probe never clears a pause raised for a still-walled Costco + (the reason renders ``... --provider ...``). Everything else — a + captcha ``challenge``, a ``needs_review`` restart pause, a crash, a spend cap, + a manual pause — lacks the marker and returns False, staying operator-gated. + """ + marker = LOGIN_PAUSE_MARKER if provider is None else f"{LOGIN_PAUSE_MARKER} {provider}" + return marker in reason + def _utcnow() -> datetime: return datetime.now(timezone.utc) diff --git a/tests/test_cli.py b/tests/test_cli.py index 60950f8..921224b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -18,7 +18,7 @@ import roomieorder.cli as cli from roomieorder.cli import _group_hits, _read_price_from_summary, main -from roomieorder.store import Store +from roomieorder.store import LOGIN_PAUSE_MARKER, Store _CATALOG = { "paper_towels": { @@ -155,6 +155,58 @@ def test_retry_unknown_row(env: Path) -> None: assert "no queue row #999" in result.output +# ─────────── login (pause-clear) ─────────── + + +class _FakeLoginPurchaser: + """Stand-in for a purchaser: no browser, reports a persisted session.""" + + domain = "www.costco.com" + profile_dir = "/tmp/profile" + + def login(self, wait_for_operator: object) -> None: # never calls the blocking cb + return None + + def verify_session(self) -> bool: + return True + + +def _pause(tmp_path: Path, reason: str) -> None: + store = Store(tmp_path / "state.sqlite") + store.init_db() + store.set_paused(True, reason) + store.close() + + +def test_login_resumes_matching_signin_pause(env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A confirmed hand-login clears the sign-in-wall pause that told the operator + to run it — closing the 'log in → still paused → logged out again' loop.""" + _pause(env, f"⚠️ Costco is logged out … run `{LOGIN_PAUSE_MARKER} costco`, then retry") + monkeypatch.setattr(cli, "_purchaser_for", lambda config, provider: _FakeLoginPurchaser()) + + result = CliRunner().invoke(main, ["login", "--provider", "costco"]) + assert result.exit_code == 0, result.output + assert "resumed" in result.output + + store = Store(env / "state.sqlite") + assert store.is_paused() is False + store.close() + + +def test_login_leaves_challenge_pause_latched(env: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A captcha challenge is not a login pause — `login` must not resume it.""" + _pause(env, "⚠️ Costco challenge on the product page — worker paused, clear it manually") + monkeypatch.setattr(cli, "_purchaser_for", lambda config, provider: _FakeLoginPurchaser()) + + result = CliRunner().invoke(main, ["login", "--provider", "costco"]) + assert result.exit_code == 0, result.output + assert "resumed" not in result.output + + store = Store(env / "state.sqlite") + assert store.is_paused() is True # still operator-gated + store.close() + + # ─────────── prune-shots ─────────── diff --git a/tests/test_main.py b/tests/test_main.py index 13b5438..ad27884 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -3,14 +3,18 @@ import time from collections.abc import Iterator from pathlib import Path +from typing import TYPE_CHECKING, cast import pytest from fastapi.testclient import TestClient +if TYPE_CHECKING: + from roomieorder.main import Engine + from roomieorder.catalog import CatalogItem from roomieorder.config import Config from roomieorder.purchase import PurchaseResult -from roomieorder.store import Status, Store +from roomieorder.store import LOGIN_PAUSE_MARKER, Status, Store class FakeOrchestrator: @@ -428,6 +432,91 @@ def verify_session(self) -> bool: engine.store.close() +def _signin_reason(provider: str) -> str: + """The pause reason a sign-in wall latches, built from the shared marker.""" + return ( + f"⚠️ {provider.capitalize()} is logged out (hit the sign-in wall on the " + f"product page) — run `{LOGIN_PAUSE_MARKER} {provider}`, then retry" + ) + + +def _logged_in_engine( + config: Config, monkeypatch: pytest.MonkeyPatch, *, signed_in: set[str] +) -> "Engine": + """An Engine whose session probe reports ``signed_in`` providers as logged in, + with the activity gate forced clear and both profiles present.""" + from roomieorder.main import Engine + + class _FakePurchaser: + def __init__(self, provider: str) -> None: + self._provider = provider + + def verify_session(self) -> bool: + return self._provider in signed_in + + monkeypatch.setattr( + "roomieorder.main.build_purchaser", + lambda cfg, provider: _FakePurchaser(provider), + ) + monkeypatch.setattr("roomieorder.main.activity.busy_gate", lambda cfg: None) + engine = Engine(config.model_copy(update={"session_check_hours": 24.0})) + engine.notifier = _RecordingNotifier() + return engine + + +def test_session_check_resumes_login_pause_when_signed_in( + config: Config, monkeypatch: pytest.MonkeyPatch +) -> None: + """A sign-in-wall pause that outlived its cause is auto-cleared once the probe + confirms the profile reloads signed in — the self-healing fix for a latched + 'Costco is logged out' that no code path otherwise resets.""" + config.costco_profile_dir.mkdir(parents=True) + config.amazon_profile_dir.mkdir(parents=True) + engine = _logged_in_engine(config, monkeypatch, signed_in={"costco", "amazon"}) + try: + engine.store.set_paused(True, _signin_reason("costco")) + engine._session_check_tick(now=1_000_000.0) + assert engine.store.is_paused() is False + sent = cast(_RecordingNotifier, engine.notifier).sent + assert any("worker resumed" in s for s in sent) + finally: + engine.store.close() + + +def test_session_check_keeps_challenge_pause_latched( + config: Config, monkeypatch: pytest.MonkeyPatch +) -> None: + """A captcha challenge needs a human — a logged-in probe must NOT auto-resume + it, even though the profile reads signed in.""" + config.costco_profile_dir.mkdir(parents=True) + config.amazon_profile_dir.mkdir(parents=True) + engine = _logged_in_engine(config, monkeypatch, signed_in={"costco", "amazon"}) + try: + engine.store.set_paused( + True, "⚠️ Costco challenge on the product page — worker paused, clear it manually" + ) + engine._session_check_tick(now=1_000_000.0) + assert engine.store.is_paused() is True + finally: + engine.store.close() + + +def test_session_check_wont_resume_other_store_pause( + config: Config, monkeypatch: pytest.MonkeyPatch +) -> None: + """A signed-in Amazon probe must not clear a pause raised for a still-walled + Costco — the resume is provider-scoped.""" + # Only amazon present, so only its probe runs; costco stays walled. + config.amazon_profile_dir.mkdir(parents=True) + engine = _logged_in_engine(config, monkeypatch, signed_in={"amazon"}) + try: + engine.store.set_paused(True, _signin_reason("costco")) + engine._session_check_tick(now=1_000_000.0) + assert engine.store.is_paused() is True + finally: + engine.store.close() + + def test_session_check_disabled_by_default(config: Config, monkeypatch: pytest.MonkeyPatch) -> None: from roomieorder.main import Engine diff --git a/tests/test_store.py b/tests/test_store.py index 73a32ac..244a531 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -2,7 +2,7 @@ from datetime import timedelta -from roomieorder.store import Store, _iso, _utcnow +from roomieorder.store import LOGIN_PAUSE_MARKER, Store, _iso, _utcnow, is_login_pause def test_enqueue_claim_mark(store: Store) -> None: @@ -110,3 +110,33 @@ def test_pause_roundtrip(store: Store) -> None: assert store.pause_reason() == "captcha" store.set_paused(False) assert store.is_paused() is False + + +# The exact reason string BasePurchaser._signin_required emits for Costco. Built +# from the shared marker so this test fails loudly if the two ever drift. +_SIGNIN_REASON = ( + "⚠️ Costco is logged out (hit the sign-in wall on the product page) — " + f"run `{LOGIN_PAUSE_MARKER} costco`, then retry" +) + + +def test_is_login_pause_recognises_signin_wall() -> None: + assert is_login_pause(_SIGNIN_REASON) is True + # Provider-scoped: matches the store the pause names… + assert is_login_pause(_SIGNIN_REASON, "costco") is True + # …and rejects a different store, so an Amazon probe can't clear a Costco pause. + assert is_login_pause(_SIGNIN_REASON, "amazon") is False + + +def test_is_login_pause_rejects_operator_gated_reasons() -> None: + # These pauses require a human — never auto-clear them. + for reason in ( + "⚠️ Costco challenge on the product page — worker paused, clear it manually", + "⚠️ 1 order(s) were interrupted by a restart (paper_towels) — review, then resume", + "worker crashed on row 7", + "⛔ recorded 24h spend $310.00 is over the $300.00 cap", + "paused via CLI", + "", + ): + assert is_login_pause(reason) is False + assert is_login_pause(reason, "costco") is False