diff --git a/CHANGELOG.md b/CHANGELOG.md index 13e138593..902a2c498 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **E2E coverage for the retired-route diagnosis** (`tests/e2e/test_retired_labs_route_diagnosis_e2e.py`, + `e2e_auth`, 0 credits, one read-only GET). The unit tests for + [#875](https://github.com/ffroliva/gflow-cli/issues/875) feed the classifier a *captured* + body, so they stay green if Flow changes the wording, un-retires the route or switches to + 401 — while the user-facing diagnosis silently goes wrong again. This one asks Flow. It + needs no project fixture (the route is retired before any project lookup) and **skips + rather than passes** on an account still served the labs tRPC API, so a cohort change + cannot retire it unnoticed. Verified by falsification: with detection disabled it fails + with the pre-fix message verbatim. + ### Fixed - **A retired labs route no longer tells you to simplify a prompt you never wrote.** diff --git a/docs/E2E_TESTING.md b/docs/E2E_TESTING.md index e06e206be..ceebcb160 100644 --- a/docs/E2E_TESTING.md +++ b/docs/E2E_TESTING.md @@ -402,9 +402,35 @@ tests/ ├── test_image_batch_e2e.py # [e2e, e2e_batch] ├── test_video_t2v_e2e.py # [e2e, e2e_video] ├── test_incident_quality_e2e.py # [e2e, e2e_auth] incident-bundle quality benchmark — 0 credits + ├── test_retired_labs_route_diagnosis_e2e.py # [e2e, e2e_auth] #875 — 0 credits, no project fixture └── test_data_layer_e2e.py # [e2e, e2e_{image,video,data}] ``` +### Diagnosing a surface Flow has retired + +`test_retired_labs_route_diagnosis_e2e.py` (marker `e2e_auth`, **0 credits**, +one read-only GET) guards what gflow *says* when a labs tRPC route answers +`404 "Flow RPCs have been deprecated and disabled"` — that it names the retired +route instead of telling the user to check a payload that is fine, simplify a +prompt that does not exist, or file a bug for a condition Flow documents in the +body being classified (#875). + +It is the rare case where the offline test genuinely cannot substitute. The unit +tests in `tests/api/test_client_errors.py` feed the classifier a *captured* body; +they stay green if Flow changes the wording, un-retires the route, or switches to +401, while the user-facing diagnosis silently goes wrong again. + +Two deliberate choices make it runnable on any profile in the nightly canary: + +- **No project fixture.** The route is retired before any project lookup, so the + diagnosis is project-independent and a syntactically valid id suffices. +- **It skips rather than passes** when the account is still served the labs tRPC + API, because a silent green would let a cohort change retire the test without + anyone noticing — the `skip_on_migrated_host` failure mode in reverse. + +Verified by falsification on 2026-09-20: with detection disabled it fails with +the pre-fix message verbatim, so it is a guard rather than a test that can only pass. + ### Incident-bundle quality benchmark `test_incident_quality_e2e.py` (marker `e2e_auth`, **0 credits**) is not a diff --git a/tests/e2e/test_retired_labs_route_diagnosis_e2e.py b/tests/e2e/test_retired_labs_route_diagnosis_e2e.py new file mode 100644 index 000000000..f08ba52aa --- /dev/null +++ b/tests/e2e/test_retired_labs_route_diagnosis_e2e.py @@ -0,0 +1,91 @@ +"""Live proof that a retired labs tRPC route diagnoses itself honestly (#875). + +Opt-in: ``-m e2e_auth`` with ``GFLOW_CLI_E2E_PROFILE`` set. One read-only GET. +**$0** — no generation, no credits, no writes to the account. + +Why this test exists as an e2e and not only as a unit test: the unit tests in +``tests/api/test_client_errors.py`` feed ``_raise_for_non_retryable`` a captured +body and assert the remediation. That proves our classifier does what we think. +It cannot prove Flow still answers the way we captured it — which is the whole +premise of this repo. If Flow un-retires the route, changes the wording, or +starts answering 401 instead, the unit tests stay green and the user-facing +diagnosis silently goes wrong again. Only a live call can tell. + +No project fixture on purpose. The route is retired *before* any project lookup +happens, so the diagnosis is project-independent and a syntactically valid id is +enough — which is what lets this run on any profile in the nightly canary with +no extra configuration. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from gflow_cli.api.client import FlowApiClient +from gflow_cli.errors import WireFormatError + +pytestmark = [pytest.mark.e2e, pytest.mark.e2e_auth] + +#: Syntactically valid, deliberately not a real project. The retired route never +#: gets as far as resolving it (measured 2026-09-20: the 404 is identical for a +#: project the account owns). +_ANY_PROJECT = "00000000-0000-4000-8000-000000000000" + +#: What v0.79.0 and earlier told the user, verbatim from ``WireFormatError``'s +#: class default. Each phrase is wrong on this response in its own way. +_PAYLOAD_CLAIM = "Check request payload parameters" +_PROMPT_CLAIM = "simpler prompt" +_BUG_CLAIM = "File a bug" + + +async def test_a_retired_labs_route_does_not_blame_the_payload_or_a_prompt( + e2e_profile_dir: Path, +) -> None: + """``character list`` is a READ. It has no prompt and no payload to simplify. + + Two outcomes are correct and this asserts the invariant across both, which is + what lets it run on a cohort we do not control and cannot choose: + + * the route answers and characters come back (an account still served the + labs tRPC API), or + * the route is retired, and the failure says so **without** telling the user + to check a payload that is fine, simplify a prompt that does not exist, or + file a bug for a condition Flow documents in the very body we classified. + """ + async with FlowApiClient(profile_dir=e2e_profile_dir) as client: + try: + characters = await client.list_characters(_ANY_PROJECT) + except WireFormatError as exc: + if "deprecated" not in (exc.detail or "").lower(): + # A different wire failure. Not this test's subject, and asserting + # on it would make an unrelated outage look like a #875 regression. + pytest.skip(f"not the retirement refusal: {exc.detail!r}") + + hint = exc.remediation_hint or "" + assert _PAYLOAD_CLAIM not in hint, f"blames a payload that is fine: {hint}" + assert _PROMPT_CLAIM not in hint, ( + f"asks to simplify a prompt that does not exist: {hint}" + ) + assert _BUG_CLAIM not in hint, f"asks for a bug report Flow already answered: {hint}" + + # It must still say something actionable rather than merely stop. + assert "projectInitialData" in hint, f"does not name the retired route: {hint}" + assert "639" in hint, f"does not point at the port that would fix it: {hint}" + + # MEASURED 2026-09-20 on denon82, both on `auto` and with the host pinned: + # GFLOW_CLI_FLOW_HOST=flow.google.com returns the SAME 404, because this + # read has no migrated arm. Suggesting it would repeat the exact failure + # #875 fixed, one field over. + assert "GFLOW_CLI_FLOW_HOST" not in hint, ( + f"suggests a setting measured not to help: {hint}" + ) + else: + # A served route is a correct outcome, but it exercised none of #875. + # Skip rather than pass: the canary runs -m e2e_auth nightly, and a + # silent green here would let a cohort change retire this test without + # anyone noticing — the failure mode `skip_on_migrated_host` was written + # for, in reverse. + assert isinstance(characters, list) + pytest.skip("labs tRPC still served for this account — #875 branch not exercised") diff --git a/website/docs/E2E_TESTING.md b/website/docs/E2E_TESTING.md index e06e206be..ceebcb160 100644 --- a/website/docs/E2E_TESTING.md +++ b/website/docs/E2E_TESTING.md @@ -402,9 +402,35 @@ tests/ ├── test_image_batch_e2e.py # [e2e, e2e_batch] ├── test_video_t2v_e2e.py # [e2e, e2e_video] ├── test_incident_quality_e2e.py # [e2e, e2e_auth] incident-bundle quality benchmark — 0 credits + ├── test_retired_labs_route_diagnosis_e2e.py # [e2e, e2e_auth] #875 — 0 credits, no project fixture └── test_data_layer_e2e.py # [e2e, e2e_{image,video,data}] ``` +### Diagnosing a surface Flow has retired + +`test_retired_labs_route_diagnosis_e2e.py` (marker `e2e_auth`, **0 credits**, +one read-only GET) guards what gflow *says* when a labs tRPC route answers +`404 "Flow RPCs have been deprecated and disabled"` — that it names the retired +route instead of telling the user to check a payload that is fine, simplify a +prompt that does not exist, or file a bug for a condition Flow documents in the +body being classified (#875). + +It is the rare case where the offline test genuinely cannot substitute. The unit +tests in `tests/api/test_client_errors.py` feed the classifier a *captured* body; +they stay green if Flow changes the wording, un-retires the route, or switches to +401, while the user-facing diagnosis silently goes wrong again. + +Two deliberate choices make it runnable on any profile in the nightly canary: + +- **No project fixture.** The route is retired before any project lookup, so the + diagnosis is project-independent and a syntactically valid id suffices. +- **It skips rather than passes** when the account is still served the labs tRPC + API, because a silent green would let a cohort change retire the test without + anyone noticing — the `skip_on_migrated_host` failure mode in reverse. + +Verified by falsification on 2026-09-20: with detection disabled it fails with +the pre-fix message verbatim, so it is a guard rather than a test that can only pass. + ### Incident-bundle quality benchmark `test_incident_quality_e2e.py` (marker `e2e_auth`, **0 credits**) is not a