From 8b538fd2af1bdc2b6ad12a56af82e370b6965dcc Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sun, 20 Sep 2026 11:08:39 +0100 Subject: [PATCH] test(e2e): ask Flow whether the retired-route diagnosis is still true (#875) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #886 fixed what gflow says when a labs tRPC route answers 404 "Flow RPCs have been deprecated and disabled", and covered it with unit tests over a captured body. Those prove our classifier does what we think. They cannot prove Flow still answers the way we captured it — and if Flow changes the wording, un-retires the route, or starts answering 401, they stay green while the user-facing remediation goes quietly wrong again. That is the gap this repo exists to close, so #886 left it as recorded debt. This pays it. The test asserts the remediation names the retired route and #639, and never tells the user to check a payload that is fine, simplify a prompt that does not exist, file a bug for a condition Flow documents in the body being classified, or set GFLOW_CLI_FLOW_HOST — measured not to help, because this read has no migrated arm. Two choices make it runnable on any profile in the nightly canary, with no extra configuration and no account writes: - No project fixture. The route is retired before any project lookup, so the diagnosis is project-independent and a syntactically valid id is enough. - It SKIPS rather than passes when the labs tRPC API still answers. A silent green would let a cohort change retire the test without anyone noticing, which is the skip_on_migrated_host failure mode in reverse. Verified live on denon82 (2026-09-20): passes in 11.9s against real Flow, and — with the detection marker disabled — fails with the pre-fix message verbatim, so it is a guard rather than a test that can only pass. Deselected by default; runs only under -m e2e_auth with GFLOW_CLI_E2E_PROFILE set. --- CHANGELOG.md | 12 +++ docs/E2E_TESTING.md | 26 ++++++ .../test_retired_labs_route_diagnosis_e2e.py | 91 +++++++++++++++++++ website/docs/E2E_TESTING.md | 26 ++++++ 4 files changed, 155 insertions(+) create mode 100644 tests/e2e/test_retired_labs_route_diagnosis_e2e.py 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