Repository navigation
fix(migrated): name the host, not the selector, when the gallery has no New project CTA - #739
Merged
Merged
Conversation
…no New project CTA "Fix the add_2 name drift" turned out to have no selector to fix. Tracing reachability before editing corrected two claims I had already made publicly on #730 and to the user: - _CHARACTER_SLOT_ADD_SELECTOR is NOT broken on migrated. _click_character_slot_add tries _CHARACTER_BODY_MODE_SELECTOR first, which already covers the migrated flow-slot-chip-button; the add_2 path is the LEGACY fallback. - ADD_MEDIA_BUTTON is unreachable there: ui_automation_video returns FlowHostMigratedError before any of its ligature selectors run. - NEW_PROJECT_SELECTORS is reached only without --project, and migrated_can_serve refuses without one. So all three are labs-only in practice and the name drift has no live production impact today. That is the third time today my count was too high, each time the same mistake: generalising from a grep without checking reachability. THE REAL DEFECT, which my own spike hit: RuntimeError: Could not find 'New project' CTA on Flow gallery. URL: https://flow.google.com/?hl=en On the migrated host that blames the SELECTOR for a control the host does not render, and its remediation - "check for a newer gflow-cli release, then file a frontend bug" - is unfixable by the reader and points at the wrong culprit. Same misdiagnosis shape as a credit shortfall reported as frontend drift (#726). Now raises FlowHostMigratedError naming the host AND the way forward (--project, which every migrated path already requires). On labs a missing CTA still reports as drift, because there it genuinely is - pinned by a test so the migrated branch cannot swallow the failure mode it was carved out of. A third test pins that the cascade still anchors on add_2, so if anyone re-anchors it on `add` the branch is revisited rather than silently kept. The safety here is GUARDS, not selectors, and guards are what a port relaxes. The measured fact now sits beside the constant: `add`, never `add_2`, with the surface counts and why a mat-icon twin would not help. Watched red first: RuntimeError: Could not find 'New project' CTA on Flow gallery. URL: https://flow.google.com/?hl=en then 3 passed. Verified: 4046 passed, 92% coverage, pyright 0 errors, ruff clean, doc links, hygiene 1031 files, website mirror. Refs #730
This was referenced Sep 7, 2026
Merged
ffroliva
added a commit
that referenced
this pull request
Sep 7, 2026
The `Release must carry /gflow:release artifacts` check was RED on #744: `docs/PROJECT_STATUS.md`'s "## Current release" still described v0.70.0. Step 9 of the protocol, and I missed it — the gate caught it, which is what it is for. The section leads with what is actually new (`--voice` verified end to end for the first time; a repo-wide grep for `--voice` across tests/e2e/ previously matched nothing) and keeps both retractions visible rather than presenting only the corrected state: #739's "the migrated gallery has no New project control", disproved by a $0 run that created one in a click, and the entity guard's "the backend rejects the generation", disproved by watching the job queue and render. Both were unproven negatives stated confidently and overturned by one cheap direct observation. A status page that shows only the end state would hide that the same error class happened twice in one release. Also carried: the #738 non-verification, whose blocker changed shape during the cycle — it is no longer credits or account access, it is retrieval.
tranzmatt
pushed a commit
to tranzmatt/gflow-cli-unofficial
that referenced
this pull request
Sep 9, 2026
…rd I got wrong Live verification overturned my own fix from an hour earlier. That is the headline. ffroliva#739 shipped a guard raising FlowHostMigratedError, asserting flow.google.com "renders no '+ New project' control gflow can drive". A $0 run of _enter_editor with no project id CREATED A PROJECT there in one click. The claim was an UNPROVEN NEGATIVE - generalised from a sweep of two other surfaces (composer, character editor) to a third that was never probed. The gallery is where NEW_PROJECT_SELECTORS actually lives, and I never measured it. That is the exact failure this session has been documenting, committed by me hours after writing the rule into repo memory. WHAT THE GALLERY ACTUALLY SHOWS (denon82, control 47 ligature nodes, $0): add* ligatures {"mat-icon|add": 1} [0] add_2 / i.gs 0 [1] add_2 / i 0 [2] add_2 / role=button 0 [3] text-matches regex ERR Error [4] has-text('New project') 1 <- the ONLY match So the ligature finding was right - `add`, never `add_2` - and the conclusion was wrong. The CTA is present and clickable; every structural Tier-1 entry missed and it survived purely on the ENGLISH TEXT fallback, which is the anti-pattern Tier 1 exists to prevent and which fails outright on a non-EN migrated profile. My guard would then have told that user the control does not exist: false, and a dead end. Fixed with a class-only `add` anchor leading Tier 1, covering both carriers. Re-measured live: entry [0] now matches 1 where every previous entry matched 0. ALSO DELETED: button:text-matches('^\+\s+\S+$', 'i'). It is not a valid Playwright selector and RAISES on every evaluation - observed twice against the live gallery - with `except Exception: continue` swallowing it. It never matched anything on any host while costing a round trip per attempt. TWO GUARD TESTS CAUGHT THIS CHANGE, AND ONE OF THEM WAS THE PROBLEM: - test_new_project_selectors_icon_leads pinned the add_2 selector exactly. Updated to the measured anchor, with the measurement as its reason. - test_new_project_selectors_plus_regex_is_anchored asserted the regex was ANCHORED - a property of a selector that never ran. It inspected the string instead of evaluating it, so it passed happily for as long as the selector was broken. Replaced with the guard that would have caught the real bug: every shipped cascade entry must PARSE against a real CSS engine (.count() on a blank page - no account, no network, no credits), plus a companion asserting the known-bad selector DOES raise, so the guard cannot quietly stop discriminating. Verified: 4069 passed, pyright 0 errors, ruff clean, doc links, hygiene, website mirror, memory round trip, PII scan - and the `add` anchor re-measured on the live gallery. Refs ffroliva#730
tranzmatt
pushed a commit
to tranzmatt/gflow-cli-unofficial
that referenced
this pull request
Sep 9, 2026
Version bumped in pyproject.toml, .codex-plugin/plugin.json and src/gflow_cli/__init__.py. CHANGELOG [Unreleased] migrated to [0.71.0] with the compare/vPREV...vNEW footer the repo already uses. Gates on this tree: repo hygiene 1034 files clean, doc links 29 resolved, website mirror in sync, website PII clean, council memory 49/49, ruff, ruff format, pyright 0 errors, and the offline suite 4070 passed / 24 skipped at 92% coverage against an 80% floor. An earlier run of that suite reported two failures and both were mine, not the release: I bumped __init__.py while it was running, and I started a scoped pytest against the shared tmp/pytest basetemp alongside it. Each passes in isolation, and the suite above is a clean re-run with nothing else touching the tree. Recording it because "two tests failed, then passed on a re-run" is exactly what a real flake looks like and this was not one. docs/LIVE_VERIFICATION_v0.71.0.md carries the 5-layer ledger, and docs/INDEX.md points at it as the latest. The headline item is that `gflow character create --voice` is verified end to end for the first time: before this release a repo-wide grep for `--voice` across tests/e2e/ matched nothing. It also settles a contradiction between two of our own documents (`sent='Charon' stored='Charon' identical=True`). This release contains a mistake and its retraction twice over, and the notes say so rather than presenting only the corrected state: - ffroliva#739 asserted the migrated gallery renders no "+ New project" control; a $0 run created a project there in one click, and ffroliva#740 anchors the CTA on `add` instead of on English text. - The entity guard was given the reason "the backend rejects it"; entity-bound generations are in fact accepted and queued, and ffroliva#742 retracts it. What fails is the observer, on a null MZZa6b payload against a 60s budget. One item is recorded as NOT verified rather than omitted: whether a bound character's voice reaches rendered audio (ffroliva#738). Attachment is proven, application is not, and the blocker is no longer credits or account access but retrieval.
5 of 7 tasks
ffroliva
added a commit
that referenced
this pull request
Sep 10, 2026
…ector Closes #756. Also fixes the 2026-09-10 RED nightly canary (#559), which is the same defect one URL apart. flow_host_kind() classifies the ORIGIN. /about, /project/<id> and /fx/api/auth/signin?error=Callback all share one, so when a readiness wait timed out it had nothing left to blame but its own anchor — sending the operator to "check for a newer release, then file a bug" over a session state no release changes. Reported at three sites and special-cased three times before (#721 credits, #749 agent mode, FlowAppError's own crash page), so it is fixed once, in the shared place, rather than a fourth time. - flow_landing_kind() joins flow_host_kind() in api/transports/_common.py and names a known non-app landing ("signin" / "public" / None). raise_for_known_landing() converts the diagnosis at the raise site. - Consulted ONLY inside an already-failed branch. Not ahead of the probe, which would delete the DOM evidence that corrects a wrong absence claim (#739 shipped one that way), and not as a bounded wait after goto, which reads page.url before Flow's client-side redirect lands (#639). - flow.google.com/about instead of the project -> FlowAppError (exit 31), naming the landing and the project it did not open, and deliberately not why: #756 measured the redirect and not its cause, and auth status reports the session verified while it happens. - labs .../auth/signin?error=Callback instead of the gallery -> AuthExpiredError (exit 3). This is the exact page behind the RED canary. - auth/internal_chromium.py drops its private _NEXTAUTH_ROUTE_PREFIX and reuses the shared classifier. The knowledge existed there since #767; no transport could reach it. Retryability was measured, not assumed. Routing /about to exit 31 would have flipped it from non-retryable to retryable purely as a side effect. The spike (scripts/dev/spike_about_redirect_stability.py, $0, ci-probe) came back 0/5 — the redirect stopped reproducing between 2026-09-08 and today — which is equally consistent with "transient" and with "a session state changed", and so settles nothing. The reading was pre-registered in the script before the run. Therefore GFlowError gains a per-instance `retryable` override (same class-default / instance-override shape as remediation_hint) and the /about raise site passes retryable=False to PRESERVE its previous exit-23 answer, not to claim a retry fails. is_retryable() pins the override with isinstance(..., bool): a MagicMock answers every attribute with a truthy child, so a truthiness test would report every mocked error as retryable with nothing noticing. Tests: tests/features/landing_state_diagnosis.feature bound from tests/e2e/test_landing_state_diagnosis_bdd.py — the first BDD-bound e2e. Three scenarios against a real browser over route interception, zero credits: both landings, plus an A/B control proving genuine drift still reports as drift. RED first (the canary's exact RuntimeError, reproduced deterministically), then green. Plus classifier unit tests and five retryability tests. tests/test_marker_registry.py now resolves a BDD module's cost tier through its Gherkin tags. It read Python source only, so it saw no pytest.mark and failed a correctly-tiered file; copying the tier into a pytestmark would have satisfied it and then drifted from the Feature.
ffroliva
added a commit
that referenced
this pull request
Sep 10, 2026
…ector (#775) * fix(transports): name a known Flow landing instead of blaming the selector Closes #756. Also fixes the 2026-09-10 RED nightly canary (#559), which is the same defect one URL apart. flow_host_kind() classifies the ORIGIN. /about, /project/<id> and /fx/api/auth/signin?error=Callback all share one, so when a readiness wait timed out it had nothing left to blame but its own anchor — sending the operator to "check for a newer release, then file a bug" over a session state no release changes. Reported at three sites and special-cased three times before (#721 credits, #749 agent mode, FlowAppError's own crash page), so it is fixed once, in the shared place, rather than a fourth time. - flow_landing_kind() joins flow_host_kind() in api/transports/_common.py and names a known non-app landing ("signin" / "public" / None). raise_for_known_landing() converts the diagnosis at the raise site. - Consulted ONLY inside an already-failed branch. Not ahead of the probe, which would delete the DOM evidence that corrects a wrong absence claim (#739 shipped one that way), and not as a bounded wait after goto, which reads page.url before Flow's client-side redirect lands (#639). - flow.google.com/about instead of the project -> FlowAppError (exit 31), naming the landing and the project it did not open, and deliberately not why: #756 measured the redirect and not its cause, and auth status reports the session verified while it happens. - labs .../auth/signin?error=Callback instead of the gallery -> AuthExpiredError (exit 3). This is the exact page behind the RED canary. - auth/internal_chromium.py drops its private _NEXTAUTH_ROUTE_PREFIX and reuses the shared classifier. The knowledge existed there since #767; no transport could reach it. Retryability was measured, not assumed. Routing /about to exit 31 would have flipped it from non-retryable to retryable purely as a side effect. The spike (scripts/dev/spike_about_redirect_stability.py, $0, ci-probe) came back 0/5 — the redirect stopped reproducing between 2026-09-08 and today — which is equally consistent with "transient" and with "a session state changed", and so settles nothing. The reading was pre-registered in the script before the run. Therefore GFlowError gains a per-instance `retryable` override (same class-default / instance-override shape as remediation_hint) and the /about raise site passes retryable=False to PRESERVE its previous exit-23 answer, not to claim a retry fails. is_retryable() pins the override with isinstance(..., bool): a MagicMock answers every attribute with a truthy child, so a truthiness test would report every mocked error as retryable with nothing noticing. Tests: tests/features/landing_state_diagnosis.feature bound from tests/e2e/test_landing_state_diagnosis_bdd.py — the first BDD-bound e2e. Three scenarios against a real browser over route interception, zero credits: both landings, plus an A/B control proving genuine drift still reports as drift. RED first (the canary's exact RuntimeError, reproduced deterministically), then green. Plus classifier unit tests and five retryability tests. tests/test_marker_registry.py now resolves a BDD module's cost tier through its Gherkin tags. It read Python source only, so it saw no pytest.mark and failed a correctly-tiered file; copying the tier into a pytestmark would have satisfied it and then drifted from the Feature. * fix(review): apply the branch-council findings 12-dimension council on 63fb5bb: 3 GREEN (correctness, auth, BDD), 9 YELLOW, 0 RED, ~19 must-fix. Two dimensions found defects in the workflow hardening itself rather than in the fix. Security (D3): the landing URL is stripped to scheme+host+path before it reaches either the message or the log. The NextAuth family the "signin" arm matches includes /fx/api/auth/callback/google?state=...&code=..., and because AuthExpiredError is capture-exempt that message is the ONLY artifact — so it is exactly what a user pastes into an issue. redact_error_detail was already imported in this module and used 300 lines below; the new raise skipped it. Truthfulness (D2, D15, D9): the message said "its sign-in page" for a family that also contains the callback and /session. docs/MCP.md still told agents FlowAppError is unconditionally retryable and that RETRYABLE_ERRORS "is the whole" list; docs/USAGE.md's exit-3 row still described only the 401/403 trigger; and a docstring claimed the helper "never adds a failure", which is false at migrated_composer.ensure_editor, whose except branch has a recovery path. Reach (D6): a THIRD site. _enter_editor(project_id=...) has no readiness gate, so a landing page reaches _locate_prompt_box and raises a bare RuntimeError, which observability.py hashes because it is not a GFlowError — the operator saw "Unexpected error" with even the URL destroyed. Worse than the report #756 is about, and the guard was not consulted there. Scope (D14, D1): retryable moves off GFlowError onto FlowAppError, the one class that needs it — is_retryable reads it by getattr, so a base field bought no typing and no test-double visibility, only a field on every error in the project. The FlowApiError pass-through it required is deleted. flow_landing_kind's "public" arm is gated to the migrated host, the only one where /about was measured. Instrument (D13): the spike re-implemented the predicate it was measuring (endswith("/about") vs the production classifier), which would score /about?hl=en as a non-reproduction — the #743 shape its own docstring cites. It now calls flow_landing_kind. except BaseException became except Exception: the former swallowed Ctrl-C, recorded it as a non-/about sample and advanced the loop, so an interrupted run could print "DOES NOT REPRODUCE" assembled from interrupts. Guards (D4, D12): the binding guard could pass on an empty scan — every check is assert-not-<list>, so a mis-resolved _FEATURES_DIR made all of them vacuous while the fire-test still passed. It now asserts it scanned something. The "one feature, one binding module" rule stated in docs was the one rule nothing enforced; it is enforced now. Feature-name matching is anchored on the path separator. The "signin" branch had zero offline coverage (only the e2e reached it, and addopts excludes that), so the 4194-pass run never executed it. Memory (D5): docs/superpowers/memory/ui-selector-drift-error-exit-23.md keeps a carve-out ledger for exceptions to exit 23 — exits 36, 37 and 38 each recorded when introduced. Carve-out 4 is now recorded, with the transferable lesson the branch is actually about: flow_host_kind classifies the ORIGIN, not the page. Hardening defects the council found in the first commit (D9): the lane said "steps 3-5 have no gate" beside "step 3 = that Gherkin RED", which is unsatisfiable when step 2 is skipped — the gate is red-before-green, never Gherkin-before-green. And the continuation table routed 1 -> 1b -> 3, orphaning Phase 2 (predict) while the routing table still mandates it for exactly this kind of change. Declined, with reasons: excluding /about from incident capture (FlowAppError also covers the React crash page, where the DOM helps, and a class cannot exclude one shape), and moving the guard after _capture_debug_screenshot (D3 showed that placement removes an existing screenshot of a signed-in page; D9's answer, taken, is to document the capture change in DEBUGGING.md rather than revert it). Verified: 4197 passed, 92% coverage; e2e 3/3 in a real browser at $0; ruff, format, doc-links, website mirror, council-memory and hygiene gates all green. * fix(transports): recognise Google's auth host as a known landing Found by live-verifying the previous commit instead of trusting it. Same profile, same test, real Google, $0: BEFORE RuntimeError: Could not find 'New project' CTA on Flow gallery. URL: https://accounts.google.com/v3/signin/accountchooser ?client_id=...&code_challenge=...&state=PKOA6qjxDh... AFTER FlowAccountChooserError: Google's account chooser is displayed (https://accounts.google.com/v3/signin/accountchooser) instead of the Flow gallery - the session needs a person to pick an account. Not selector drift. `flow_landing_kind` returned None for accounts.google.com by design, and a unit test asserted it, on the reasoning "the chooser has its own handler". That is true at BOOTSTRAP - `client._handle_account_chooser` - and false for a hop that happens after it, which is what profile `denon82` did on 2026-09-10: it left the `?error=Callback` state the 02:00 canary reported and moved to Google's chooser, where `_enter_editor` swept eighteen selectors for a "+ New project" CTA on Google's sign-in page and blamed the anchor. So the previous commit's claim to fix the RED canary was false for the state the profile is actually in. The classifier now answers for Google's auth host too: "chooser" for a `/v3/signin/accountchooser` path -> FlowAccountChooserError (38, the class #763 and #764 already established for this), "signin" for its other sign-in surfaces -> AuthExpiredError (3). The bot-rejection hop (`/v3/signin/rejected`) keeps returning None; it has its own error and must never read as a missing account. Path-tested rather than importing GOOGLE_REJECTED_BROWSER_ROUTE, because `_common` -> `auth` is a real import cycle - which is why `internal_chromium` imports this module deferred. `_is_safe_to_probe_session` is unaffected, verified: its `flow_host_kind(url) is None` short-circuit returns False for accounts.google.com before this function is consulted, so the cookie-rotation gate the council proved equivalent over 393 cases still behaves identically. D3's redaction finding is no longer hypothetical - that live message carried the OAuth `state`, `code_challenge` and `client_id`, and this is the artifact users paste into issues. The stripping added in 7e29be4 removes them. Not in scope, and recorded rather than smuggled in: the mid-run chooser now DIAGNOSES but does not RECOVER. #764 taught the bootstrap path to autoselect the recorded account; that lives on FlowApiClient and is not reachable from a transport helper. Teaching this hop to autoselect would rescue the run instead of naming the failure. Verified: 4202 passed, 92% coverage; the live A/B above; ruff, format, doc-links, website mirror and hygiene gates green. * docs(changelog): fold the duplicate Added heading the rebase created Rebasing onto develop after #774 squash-merged stacked this branch's Added section above the one that arrived with the hardening, leaving two under [Unreleased]. Keep a Changelog wants one per type; content is unchanged.
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
"Fix the
add_2name drift" turned out to have no selector to fix. Tracing reachability before editing corrected two claims I had already made publicly on #730.Refs #730
The correction
_CHARACTER_SLOT_ADD_SELECTOR_click_character_slot_addtries_CHARACTER_BODY_MODE_SELECTORfirst, which already covers the migratedflow-slot-chip-button; theadd_2path is the legacy fallbackADD_MEDIA_BUTTONui_automation_video.pyreturnsFlowHostMigratedErrorbefore any of its ligature selectors runNEW_PROJECT_SELECTORS--project— andmigrated_can_serverefuses without oneAll three are labs-only in practice, so the name drift has no live production impact today. That was the third time in this session my count was too high, each time the same mistake: generalising from a grep without checking reachability.
The real defect, which my own spike hit
On the migrated host this blames the selector for a control that host does not render. Its remediation — "check for a newer gflow-cli release, then file a frontend bug" — is unfixable by the reader and points at the wrong culprit. The same misdiagnosis shape as a credit shortfall reported as frontend drift (#726).
It now raises
FlowHostMigratedErrornaming the host and the way forward (--project, which every migrated path already requires).Why this is worth shipping when nothing reaches it today
The
add_2drift is harmless because of guards —migrated_can_serverefusing without a project id,ensure_editornavigating directly,character createrequiring--project. Guards are exactly what a port relaxes. A spike hit this message on 2026-09-07 and learned nothing from it; the next person to un-guard a path would get the same.So the measured fact now sits beside the constant:
add, neveradd_2(composeradd=1/add_2=0, editoradd=2/add_2=0, per-surface controls passing), including why amat-icontwin would not have helped — this is a ligature name drift, not the carrier split #735 fixed.Tests
Watched red first:
Three tests, then green:
FlowHostMigratedErrornaming the host and--projectadd_2— if anyone re-anchors it onadd, this fails and the branch gets revisited rather than silently keptValidation
pytest -q --cov— 4046 passed, 7 skipped, 92% coveragepyright src0 errors ·ruff check/format --checkcleanE2E: none owed. This changes an error message on a path no production flow reaches on the migrated host, and leaves the labs path byte-identical. The behaviour is pinned by unit tests on both branches; a live run would exercise neither, since reaching it requires deliberately calling
_enter_editorwithout a project id on a migrated account — which is what the spike did, and what produced the misleading message in the first place.