diff --git a/CHANGELOG.md b/CHANGELOG.md index 69ea77aa3..d6fc3c40b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A corrupt cookie store is no longer reported as a network problem** + ([#796](https://github.com/ffroliva/gflow-cli/issues/796)). When a profile's Chrome + cookie database could not be read as a database, `gflow auth status` said "check + network connectivity". It now names the cookie store and says to close any Chrome + window using the profile and run `gflow auth login` again. The MCP `gflow_auth_status` + tool returns a new `cookie_store_unreadable` outcome (HTTP 409, `retryable: false`) + instead of the retryable `verification_error` (#796). + - **The MCP `wait` parameter is described where agents read it**. `gflow_generate_image` and `gflow_generate_video` now carry a `wait` description in `tools/list`: the default `true` runs the generation inside the call, which can outlast a client's tool timeout, diff --git a/docs/MCP.md b/docs/MCP.md index 1e9a4a6a5..04715d277 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -109,7 +109,7 @@ The server registers three protocol surfaces: > until a worker ships ([#1097](https://github.com/ffroliva/gflow-cli/issues/1097)). -* `gflow_auth_status(profile)`: Credit-free, non-interactive Flow session probe (#497) — wraps the same fail-closed `verify_flow_profile` check as `gflow auth status`. Returns `{"status": "authenticated", "profile", "user_email"}` or a problem-details error with a `remediation_hint`; a `verification_error` outcome (HTTP 503, `retryable: true`) means a network/endpoint problem, which re-login does not fix, and a `profile_marker_missing` outcome (HTTP 409, `retryable: false`, #796) means the profile lost its `.gflow_browser_strategy` marker so its cookies cannot be read — neither a retry nor a re-login helps; re-run `gflow auth login --browser chrome` for that profile. Any other unauthenticated outcome is a dead session (HTTP 401). Call it before a generation tool to fail fast on dead auth, before a long blocking generation call. Login/logout remain CLI-only (genuinely interactive). +* `gflow_auth_status(profile)`: Credit-free, non-interactive Flow session probe (#497) — wraps the same fail-closed `verify_flow_profile` check as `gflow auth status`. Returns `{"status": "authenticated", "profile", "user_email"}` or a problem-details error with a `remediation_hint`; a `verification_error` outcome (HTTP 503, `retryable: true`) means a network/endpoint problem, which re-login does not fix, and a `profile_marker_missing` outcome (HTTP 409, `retryable: false`, #796) means the profile lost its `.gflow_browser_strategy` marker so its cookies cannot be read — neither a retry nor a re-login helps; re-run `gflow auth login --browser chrome` for that profile. A `cookie_store_unreadable` outcome (HTTP 409, `retryable: false`, #796) means the profile's Chrome cookie database is corrupt or unreadable — a local file fault, not the network; close any Chrome window using that profile, then re-run `gflow auth login` for it. Any other unauthenticated outcome is a dead session (HTTP 401). Call it before a generation tool to fail fast on dead auth, before a long blocking generation call. Login/logout remain CLI-only (genuinely interactive). * `gflow_get_credits(profile, all_profiles, refresh)`: Current Flow balance query; spends no credits. With `all_profiles=true`, inspects every saved profile sequentially and preserves successful balances plus `total_credits` when another profile fails. The balance is read from flow.google.com in the profile's browser: the number the app itself loads, which is the one Flow's account menu shows (#795). That is one browser launch per profile, and the tier fields (`sku`, `service_tier`, `user_paygate_tier`, `subscription_credits`) are always `null`. With `refresh=true`, a profile that lands on a sign-in page is renewed in a headless browser and read again: one more Chrome launch per such profile, no credits. This is the MCP twin of `gflow credits user|list --refresh`. It runs the same renewal as `gflow auth renew`, but only for a profile whose credits read lands on a sign-in page. `auth renew` itself has no MCP tool; see the exemption in `tests/mcp/test_cli_parity.py`. Without `refresh` the call is read-only; with it, an expired profile's saved cookies are rewritten. This tool spends no credits and returns no cookies, bearer tokens, or browser API keys. The balance funds Veo video generation; image generation uses separate per-model daily quotas. A page that loads but never fetches the balance is a `FlowAppError` (exit 31), never an auth error. * `gflow_character_list(project, profile)`: Lists a project's saved Flow CHARACTER entities with their **entity ids** — read-only, spends no credits, drives a browser session. This is how an agent discovers what it can attach: an `entity_id` goes to `reference_entities`, a `display_name` can be used as an `@Name` mention. An empty list means the project genuinely has none. * `gflow_character_show(project, entity_id, name, profile)`: Shows one character by id or exact display name (exactly one selector required). An ambiguous name is refused rather than resolved arbitrarily — which is the reason to prefer the id. Read-only; drives a browser session. diff --git a/src/gflow_cli/auth/internal_chromium.py b/src/gflow_cli/auth/internal_chromium.py index cf6b44ad5..64a91b067 100644 --- a/src/gflow_cli/auth/internal_chromium.py +++ b/src/gflow_cli/auth/internal_chromium.py @@ -58,6 +58,9 @@ FlowSessionOutcome.PROFILE_MARKER_MISSING: ( "This profile is missing its Chrome-strategy marker, so its cookies cannot be read." ), + FlowSessionOutcome.COOKIE_STORE_UNREADABLE: ( + "This profile's cookie store is corrupt or unreadable." + ), } UNVERIFIED_HINT: dict[FlowSessionOutcome, str] = { FlowSessionOutcome.GOOGLE_SESSION_ONLY: ( @@ -71,6 +74,11 @@ FlowSessionOutcome.PROFILE_MARKER_MISSING: ( "Re-run `gflow auth login --browser chrome` to rewrite the profile marker." ), + FlowSessionOutcome.COOKIE_STORE_UNREADABLE: ( + "Close any Chrome window using this profile, then re-run `gflow auth login`. " + "If it repeats, delete the profile's `Default/Network/Cookies` file (this signs " + "the profile out) and log in again." + ), } diff --git a/src/gflow_cli/auth/verification.py b/src/gflow_cli/auth/verification.py index 7b6587ef2..21840ac89 100644 --- a/src/gflow_cli/auth/verification.py +++ b/src/gflow_cli/auth/verification.py @@ -15,6 +15,7 @@ import asyncio import json import re +import sqlite3 from collections import Counter from dataclasses import dataclass from enum import StrEnum @@ -72,6 +73,10 @@ class FlowSessionOutcome(StrEnum): #: network — and telling the user to "check connectivity" sends them #: looking in the wrong place, on a profile a failed login just rolled back. PROFILE_MARKER_MISSING = "profile_marker_missing" + #: Chrome's cookie DB raised `sqlite3.DatabaseError` (corrupt, or not a + #: database at all), so it cannot be read (#796). Local file state, not the + #: network — and not retried through Playwright, which would raze the file. + COOKIE_STORE_UNREADABLE = "cookie_store_unreadable" _DETAIL_BY_OUTCOME: dict[FlowSessionOutcome, str] = { @@ -82,6 +87,9 @@ class FlowSessionOutcome(StrEnum): FlowSessionOutcome.PROFILE_MARKER_MISSING: ( "This profile is missing its Chrome-strategy marker." ), + FlowSessionOutcome.COOKIE_STORE_UNREADABLE: ( + "This profile's cookie store is corrupt or unreadable." + ), } @@ -564,6 +572,16 @@ async def verify_flow_profile( source=source, ) + # #796: same misdirection, different trigger — a corrupt cookie DB on the + # user's own disk was reported as "check network connectivity". + except sqlite3.DatabaseError as exc: + logger.warning("auth_cookie_store_unreadable", source=source, error=type(exc).__name__) + return FlowSessionStatus( + outcome=FlowSessionOutcome.COOKIE_STORE_UNREADABLE, + user_email=None, + source=source, + ) + # Fail-closed: any failure here yields VERIFICATION_ERROR, never AUTHENTICATED. except Exception as exc: logger.warning("auth_flow_session_probe_error", source=source, error=type(exc).__name__) diff --git a/src/gflow_cli/cli.py b/src/gflow_cli/cli.py index 6f5edc3db..3ce0c34f3 100644 --- a/src/gflow_cli/cli.py +++ b/src/gflow_cli/cli.py @@ -387,6 +387,13 @@ def auth_status(profile: str | None) -> None: f"Re-run [bold]gflow auth login --browser chrome --profile {name}[/bold] " "to rewrite it.", ) + elif status_result.outcome is verification.FlowSessionOutcome.COOKIE_STORE_UNREADABLE: + # #796: a corrupt cookie DB on disk, not the network. + console.print( + f"[yellow]{escape(str(status_result.detail))}[/yellow] " + "Close any Chrome window using this profile, then re-run " + f"[bold]gflow auth login --profile {name}[/bold].", + ) elif status_result.outcome is verification.FlowSessionOutcome.VERIFICATION_ERROR: # Re-login cannot fix an unreachable endpoint — don't send the # user into an interactive browser flow for a network problem. diff --git a/src/gflow_cli/mcp/tools.py b/src/gflow_cli/mcp/tools.py index 3acfdff9f..1a43af0f7 100644 --- a/src/gflow_cli/mcp/tools.py +++ b/src/gflow_cli/mcp/tools.py @@ -1907,6 +1907,22 @@ async def gflow_auth_status(profile: str = _DEFAULT_PROFILE) -> dict[str, Any]: "chrome` for this profile to rewrite it." ), } + elif status.outcome is verification.FlowSessionOutcome.COOKIE_STORE_UNREADABLE: + # #796: local file state like the marker case — a retry re-reads the + # same broken file, so not retryable, and not a network 503. + error = { + "type": "https://gflow-cli.dev/errors/cookie-store-unreadable", + "title": "Profile cookie store is unreadable", + "status": 409, + "detail": status.detail, + "message": status.detail, + "retryable": False, + "remediation_hint": ( + "This profile's Chrome cookie database is corrupt or unreadable. " + "Close any Chrome window using this profile, then re-run " + "`gflow auth login` for it." + ), + } elif status.outcome is verification.FlowSessionOutcome.VERIFICATION_ERROR: # A network/endpoint problem is not fixed by re-login — the # machine-readable discriminators must say so too (post-merge review: diff --git a/tests/auth/strategies/test_strategies.py b/tests/auth/strategies/test_strategies.py index 0eb2ae029..de139d36c 100644 --- a/tests/auth/strategies/test_strategies.py +++ b/tests/auth/strategies/test_strategies.py @@ -190,6 +190,7 @@ async def _verify(*_a: object, **_k: object) -> FlowSessionStatus: FlowSessionOutcome.GOOGLE_SESSION_ONLY, FlowSessionOutcome.NO_SESSION, FlowSessionOutcome.VERIFICATION_ERROR, + FlowSessionOutcome.COOKIE_STORE_UNREADABLE, # #796: no KeyError on login ], ) async def test_real_chrome_unverified_raises_auth_missing( diff --git a/tests/auth/test_verification.py b/tests/auth/test_verification.py index 84d32735d..74f5cb22b 100644 --- a/tests/auth/test_verification.py +++ b/tests/auth/test_verification.py @@ -343,6 +343,70 @@ class BrowserCookieError(Exception): assert not status.authenticated fake_async_playwright.assert_not_called() + @pytest.mark.skipif( + sys.platform != "win32", + reason=( + "browser_cookie3 reads the keyring key before opening the DB on Linux/macOS; " + "without a keyring (CI) it falls back first and never reaches the corrupt file" + ), + ) + @pytest.mark.asyncio + async def test_corrupt_cookie_store_is_not_reported_as_a_network_fault( + self, gflow_home: Path + ) -> None: + """#796: a corrupt cookie DB raises sqlite3.DatabaseError from the real + browser_cookie3 read. It is local profile state, so it must not land in + VERIFICATION_ERROR ("check network connectivity"), nor launch the + Playwright fallback — Chrome would raze the file during a status probe.""" + profile = gflow_home / "profile_default" + cookies = profile / "Default" / "Network" / "Cookies" + cookies.parent.mkdir(parents=True) + cookies.write_bytes(b"SQLite format 3\x00" + b"\xa5" * 4080) # real file, not a mock + (profile / ".gflow_browser_strategy").write_text("chrome", encoding="utf-8") + fake_async_playwright = MagicMock() + + with ( + patch("gflow_cli.auth.verification.get_settings", **NO_PROXY) as mock_settings, + patch("gflow_cli.auth.strategies.async_playwright", fake_async_playwright), + ): + mock_settings.return_value.home = gflow_home + status = await verify_flow_profile(profile) + + assert status.outcome is FlowSessionOutcome.COOKIE_STORE_UNREADABLE + assert not status.authenticated + assert "network" not in status.detail.lower() + fake_async_playwright.assert_not_called() + + @pytest.mark.asyncio + async def test_a_database_error_from_the_cookie_read_is_unreadable_on_every_os( + self, gflow_home: Path + ) -> None: + """#796, portable: the exception the real corrupt file raises on Windows, + injected where browser_cookie3 raises it, classifies the same everywhere.""" + import sqlite3 + + profile = gflow_home / "profile_default" + cookies = profile / "Default" / "Network" / "Cookies" + cookies.parent.mkdir(parents=True) + cookies.write_bytes(b"x") + (profile / ".gflow_browser_strategy").write_text("chrome", encoding="utf-8") + fake_async_playwright = MagicMock() + + with ( + patch("gflow_cli.auth.verification.get_settings", **NO_PROXY) as mock_settings, + patch("gflow_cli.auth.strategies.async_playwright", fake_async_playwright), + patch( + "browser_cookie3.chrome", + side_effect=sqlite3.DatabaseError("file is not a database"), + ), + ): + mock_settings.return_value.home = gflow_home + status = await verify_flow_profile(profile) + + assert status.outcome is FlowSessionOutcome.COOKIE_STORE_UNREADABLE + assert "network" not in status.detail.lower() + fake_async_playwright.assert_not_called() + @pytest.mark.asyncio async def test_verification_error_on_browser_cookie_decryption_error( self, gflow_home: Path diff --git a/tests/cli/test_auth_status.py b/tests/cli/test_auth_status.py index bc923115d..5241ed494 100644 --- a/tests/cli/test_auth_status.py +++ b/tests/cli/test_auth_status.py @@ -6,6 +6,7 @@ from __future__ import annotations +import sys from pathlib import Path import pytest @@ -83,6 +84,29 @@ def test_exit_1_on_verification_error(monkeypatch: pytest.MonkeyPatch) -> None: assert "connectivity" in result.output +@pytest.mark.skipif( + sys.platform != "win32", + reason=( + "browser_cookie3 reads the keyring key before opening the DB on Linux/macOS; " + "without a keyring (CI) it falls back first and never reaches the corrupt file" + ), +) +def test_corrupt_cookie_store_is_not_blamed_on_the_network() -> None: + """#796, end to end through the real probe: a corrupt cookie DB on disk + (no mock of verify_flow_profile or browser_cookie3) must not tell the user to + check network connectivity — the fault is the file on their own disk.""" + pdir = get_settings().profile_subdir("corrupt") + pdir.mkdir(parents=True, exist_ok=True) + (pdir / "Cookies").write_bytes(b"SQLite format 3\x00" + b"\xa5" * 4080) + + result = CliRunner().invoke(main, ["auth", "status", "--profile", "corrupt"]) + + assert result.exit_code == 1 + assert "connectivity" not in result.output + assert "cookie store" in result.output.lower() + assert "gflow auth login --profile corrupt" in result.output + + def test_bracketed_email_renders_without_markup_crash(monkeypatch: pytest.MonkeyPatch) -> None: """A server-supplied email is untrusted text — Rich markup must be escaped (this repo's known bracketed-text crash class).""" diff --git a/tests/mcp/test_auth_status_tool.py b/tests/mcp/test_auth_status_tool.py index fe20c5d1c..7d789ad43 100644 --- a/tests/mcp/test_auth_status_tool.py +++ b/tests/mcp/test_auth_status_tool.py @@ -103,3 +103,25 @@ async def test_a_missing_profile_marker_is_not_retryable_and_not_a_dead_session( assert result["error"]["retryable"] is False assert result["error"]["status"] == 409 assert "--browser chrome" in result["error"]["remediation_hint"] + + +async def test_an_unreadable_cookie_store_is_not_retryable_and_not_a_network_fault( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """#796 + MCP parity: a corrupt cookie DB is local profile state. A retry + re-reads the same broken file, so it is not retryable, and it is not the + 503 network/endpoint envelope an agent would retry on.""" + from gflow_cli.mcp import tools + + monkeypatch.setattr(tools, "_resolve_and_validate_profile", lambda p: "denon") + monkeypatch.setattr( + tools.verification, + "verify_flow_profile", + _probe(FlowSessionOutcome.COOKIE_STORE_UNREADABLE), + ) + result = await tools.gflow_auth_status() + assert result["status"] == "cookie_store_unreadable" + assert result["error"]["retryable"] is False + assert result["error"]["status"] == 409 + assert "connectivity" not in result["error"]["remediation_hint"] + assert "gflow auth login" in result["error"]["remediation_hint"] diff --git a/website/docs/MCP.md b/website/docs/MCP.md index 1e9a4a6a5..04715d277 100644 --- a/website/docs/MCP.md +++ b/website/docs/MCP.md @@ -109,7 +109,7 @@ The server registers three protocol surfaces: > until a worker ships ([#1097](https://github.com/ffroliva/gflow-cli/issues/1097)). -* `gflow_auth_status(profile)`: Credit-free, non-interactive Flow session probe (#497) — wraps the same fail-closed `verify_flow_profile` check as `gflow auth status`. Returns `{"status": "authenticated", "profile", "user_email"}` or a problem-details error with a `remediation_hint`; a `verification_error` outcome (HTTP 503, `retryable: true`) means a network/endpoint problem, which re-login does not fix, and a `profile_marker_missing` outcome (HTTP 409, `retryable: false`, #796) means the profile lost its `.gflow_browser_strategy` marker so its cookies cannot be read — neither a retry nor a re-login helps; re-run `gflow auth login --browser chrome` for that profile. Any other unauthenticated outcome is a dead session (HTTP 401). Call it before a generation tool to fail fast on dead auth, before a long blocking generation call. Login/logout remain CLI-only (genuinely interactive). +* `gflow_auth_status(profile)`: Credit-free, non-interactive Flow session probe (#497) — wraps the same fail-closed `verify_flow_profile` check as `gflow auth status`. Returns `{"status": "authenticated", "profile", "user_email"}` or a problem-details error with a `remediation_hint`; a `verification_error` outcome (HTTP 503, `retryable: true`) means a network/endpoint problem, which re-login does not fix, and a `profile_marker_missing` outcome (HTTP 409, `retryable: false`, #796) means the profile lost its `.gflow_browser_strategy` marker so its cookies cannot be read — neither a retry nor a re-login helps; re-run `gflow auth login --browser chrome` for that profile. A `cookie_store_unreadable` outcome (HTTP 409, `retryable: false`, #796) means the profile's Chrome cookie database is corrupt or unreadable — a local file fault, not the network; close any Chrome window using that profile, then re-run `gflow auth login` for it. Any other unauthenticated outcome is a dead session (HTTP 401). Call it before a generation tool to fail fast on dead auth, before a long blocking generation call. Login/logout remain CLI-only (genuinely interactive). * `gflow_get_credits(profile, all_profiles, refresh)`: Current Flow balance query; spends no credits. With `all_profiles=true`, inspects every saved profile sequentially and preserves successful balances plus `total_credits` when another profile fails. The balance is read from flow.google.com in the profile's browser: the number the app itself loads, which is the one Flow's account menu shows (#795). That is one browser launch per profile, and the tier fields (`sku`, `service_tier`, `user_paygate_tier`, `subscription_credits`) are always `null`. With `refresh=true`, a profile that lands on a sign-in page is renewed in a headless browser and read again: one more Chrome launch per such profile, no credits. This is the MCP twin of `gflow credits user|list --refresh`. It runs the same renewal as `gflow auth renew`, but only for a profile whose credits read lands on a sign-in page. `auth renew` itself has no MCP tool; see the exemption in `tests/mcp/test_cli_parity.py`. Without `refresh` the call is read-only; with it, an expired profile's saved cookies are rewritten. This tool spends no credits and returns no cookies, bearer tokens, or browser API keys. The balance funds Veo video generation; image generation uses separate per-model daily quotas. A page that loads but never fetches the balance is a `FlowAppError` (exit 31), never an auth error. * `gflow_character_list(project, profile)`: Lists a project's saved Flow CHARACTER entities with their **entity ids** — read-only, spends no credits, drives a browser session. This is how an agent discovers what it can attach: an `entity_id` goes to `reference_entities`, a `display_name` can be used as an `@Name` mention. An empty list means the project genuinely has none. * `gflow_character_show(project, entity_id, name, profile)`: Shows one character by id or exact display name (exactly one selector required). An ambiguous name is refused rather than resolved arbitrarily — which is the reason to prefer the id. Read-only; drives a browser session.