Repository navigation
feat: add Flow credit balance commands - #671
Conversation
|
Thanks for the contribution. This PR has been marked as an external contribution and routed for maintainer review. Before merge, please make sure:
GitHub Copilot code review may be requested as an advisory first pass, but maintainer approval is still required. |
Maintainer council review — verdict 🔴 RED (reviewed at
|
…er clause to the merge Council review of #677 found the substitution still reachable -- not through the wording, but through the verdict colour. D4 said "must-fix" while scoring YELLOW, and a YELLOW is dismissable under section 5 step 8 with a one-line justification. "The live runs in the PR body are good enough" is precisely the justification someone writes, which reopens the ledger-for-e2e substitution through the escape valve rather than through the text. - Absent with no reason given now scores D4 RED, forcing consensus RED under section 5 step 3, and the skill says why it is not YELLOW so nobody softens it back later. - Absent WITH a stated reason stays YELLOW, blocked on a maintainer run -- that is the documented exemption, not an evasion. - Present is now verified rather than trusted: cite the tests/e2e file:line, confirm it exists at REVIEWED_SHA, confirm it exercises the changed surface. A passing e2e test that never touches the changed path is pr-must-verify-on-affected-surface wearing an e2e label. - The grandfather clause said "PRs opened before 2026-09-05", which excluded the only two PRs it exists for: #669 opened 11:28Z and #671 at 13:38Z, both hours before #675 merged at 19:15Z the same day. Now anchored to the merge commit, not the calendar day. The published slug carries the same RED wording, so the memory a future reviewer loads cannot disagree with the skill that cites it.
73c57db to
a46188d
Compare
E2E is the layer that proves a change works against the real Flow blackbox; offline-green only proves our code does what we think it does. Nothing in the contributor path asked for it: the lifecycle table routed only generation-path changes to live-verify, "Test categories" documented how to run e2e but framed it as opt-in maintainer regression, and the PR template's only live-Flow line was scoped the same way. External PR ffroliva#671 followed all of it to the letter and arrived with eight offline test files, zero e2e -- and its HTTP fast path failed on the first real run. Also records what live-verify is not: it drives CLI commands by hand into a gitignored note and never runs `pytest -m e2e`. An e2e test is a re-runnable regression; a live-verify ledger is a narrative record. Both, not either. - CONTRIBUTING.md: new lifecycle row for any behavior change; fourth pipeline rule; "E2E is required, not optional" in the test-categories section - AGENTS.md: same rule where agents read it, plus the cost sub-marker list - .github/PULL_REQUEST_TEMPLATE.md: an e2e line that has to be answered Drive-by: AGENTS.md said "six gates above" over a nine-command block.
Maintainer council review — verdict 🟢 GREEN (reviewed at
|
| Item | Evidence at head |
|---|---|
| #2 cookie jar sent cross-host | The labs.google session client is closed before a separate bare, header-only client makes the aisandbox call (api/credits.py:71). No jar crosses the origin boundary. |
| #3 no retry on the session probe | Both callers now share fetch_flow_session_httpx, which keeps _MAX_ATTEMPTS = 3 and the 429/503/504 backoff. The duplication is gone as well. |
| #4 docs contradiction + broken anchor | docs/USER_GUIDE.md:409 rewritten; the dead #no-in-cli-quota-visibility fragment removed; website/docs mirror regenerated and in sync. |
| #5 agent-facing surfaces | credits added to the AGENTS.md command surface, cli_credits.py to the module inventory, plus llms.txt and docs/INDEX.md. |
| #6 test gaps on the default UX path | api/credits.py 100%, cli_credits.py 100%, services/credits.py 98% (43 tests). _render_one / _render_many were 0% and are now fully covered. |
| #7 no e2e test | tests/e2e/test_credits_e2e.py, marked e2e + e2e_auth, gated on GFLOW_CLI_E2E_PROFILE. Ran live here: 1 passed in 1.48s. It patches the Playwright cookie fallback to fail immediately, so a green result is the browser-free proof — exactly the check that would have caught #1 before the PR was opened. |
Nice-to-haves 1, 2, 3, 5, 6 and 7 are all in as well: _failure now derives its keys from dataclasses.fields(CreditsInfo), the CLI is trimmed to credits user [--profile] + credits list with no swallowed group flags, _validate_profile_in_home guards the shared probe, the conditional-expression statements are if/else, and the video-only nature of this balance is stated in the user guide, KNOWN_ISSUES and llms.txt.
Gates at head
check_repo_hygiene · check_doc_links · check_website_docs_pii · generate_website_docs --check · check_council_memory · ruff check · ruff format --check — all green. pyright src — 0 errors, 0 warnings. Rebased cleanly onto develop after v0.68.0 with the CHANGELOG entry still under [Unreleased], so the merge-order note is resolved.
SonarCloud is skipped on fork PRs, so this maintainer review is the substitute server-side gate.
One residual, not a blocker
verify_flow_profile now validates the profile path twice — once itself, once inside the extracted helper. Harmless and cheap, and the outer call deliberately stays outside the fail-closed except Exception so a SecurityError still propagates rather than being downgraded to VERIFICATION_ERROR.
Thanks for the thorough turnaround — particularly for adding the e2e test that makes the central claim self-proving rather than asserted.
|
Merged as Thank you for this. A few things about how you handled it are worth naming, because they are not the norm: You fixed the blocker rather than arguing with it. The RED said the HTTP fast path fell back to a real Chrome launch on every profile on my machine, which is a hard thing to receive when it passed on yours. You went and found the cross-host cookie leak underneath it instead of asking me for more logs. Re-tested here on the same machine that produced the RED: You extracted the shared session probe instead of copying the retry block. The e2e test is the part I will be pointing other contributors at. Patching the Playwright cookie fallback to fail immediately means a green run is the browser-free proof, not an assertion about one. This project reverse-engineers a blackbox — offline-green only ever proves our code does what we think it does, never that Flow still behaves as captured — so a test that makes its own central claim falsifiable is exactly the right shape. It is also, precisely, the test that would have caught the blocker before the PR was opened. And you took all seven nice-to-haves, including the one that was really a documentation problem: the note that this balance funds Veo video while image generation draws on separate per-model daily quotas. Two currencies get conflated constantly and now the docs say so in three places.
Ships in the next release. Thanks again — genuinely good work. |
Summary
gflow credits user/gflow credits listcommands and the matchinggflow_get_creditsMCP toollabs.googlecookie client is closed before a separate cookie-free, Bearer-only credits requestReview fixes
developafter v0.68.0 and kept the changelog entry under[Unreleased]credits userandcredits listVerification
uv run ruff check src testsuv run ruff format --check src testsuv run pyright src— 0 errors, 0 warningscredits,serviceTier,sku,subscriptionCredits,userPaygateTier; unknown key count 0; 1 passed in 1.49sa46188d1e1c6ef416c6780f05cebcd4688da4b4bThe live e2e patches the Playwright cookie fallback to raise immediately, so its passing result confirms the credits request completed through the HTTP fast path without launching a browser.