test(e2e): cover label-scoped budgets in the release curl matrix - #597
Conversation
Adds S197-S204 for budgets scoped to a request label rather than a user_path subtree: admin CRUD and validation negatives, tagging-header enforcement with verbatim case-sensitive matching, multi-label charging, managed-key label charging, a request matched by both a user_path and a label budget, reset-one, PostgreSQL/MongoDB parity, and auth gating. S200 runs on the auth-enabled gateway rather than the main SQLite one: creating a managed key on a gateway with no GOMODEL_MASTER_KEY switches every endpoint to require bearer auth, and managed keys have no delete endpoint, so registering one there would lock the gateway down for every later scenario. Two scenarios promised more than they checked, so the assertions were brought up to the prose: S199 now checks the block names the exhausted label and *not* the healthy one, and S201 checks the user-path budget that matched the same request was charged and still has room — which is what makes it evidence that the label budget alone caused the block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe release E2E scenario matrix now covers 204 scenarios. New scenarios S197–S204 document label-scoped budget administration, enforcement, matching, reset behavior, backend parity, and authentication gating. ChangesLabel-scoped budget E2E coverage
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/release-e2e-scenarios.md`:
- Around line 4793-4796: Update the S197–S204 preamble to distinguish scenarios
that use tagging headers from admin-only scenarios and S200’s managed-key
labels. State that only scenarios configuring tagging rules restore the tagging
rule set, while preserving the existing self-contained cleanup description for
budgets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3df02967-70d3-48ae-a371-57be731406db
📒 Files selected for processing (1)
tests/e2e/release-e2e-scenarios.md
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Confidence Score: 3/5The stateful scenario cleanup needs to survive assertion failures before this PR is safe to merge. The new scenarios create tiny daily budgets and global tagging rules but clean them up only at the end; an earlier command failure leaves spent state that causes documented same-suffix reruns to fail immediately. Files Needing Attention: tests/e2e/release-e2e-scenarios.md
What T-Rex did
Reviews (1): Last reviewed commit: "test(e2e): cover label-scoped budgets in..." | Re-trigger Greptile |
| curl -fsS -X PUT "$BASE_URL/admin/tagging/settings" \ | ||
| -H 'Content-Type: application/json' -d "{\"headers\":[{\"header\":\"$TAG_HDR\"}]}" >/dev/null | ||
| curl -fsS -X PUT "$BASE_URL/admin/budgets" \ | ||
| -H 'Content-Type: application/json' \ | ||
| -d "{\"scope\":\"label\",\"subject\":\"$QA_LBL\",\"budget_key\":{\"period\":\"daily\"},\"amount\":$QA_BUDGET_AMOUNT}" >/dev/null |
There was a problem hiding this comment.
Failed scenarios retain budget state
When S198–S203 exit on any request or assertion failure, set -e skips their end-of-scenario budget and tagging cleanup. Rerunning with the documented same QA_SUFFIX then reuses an already-spent tiny daily budget, causing the first request to return 429 and making the release scenario fail repeatedly until that state is manually cleared.
Review follow-ups. Budget spend is a SUM over usage rows, so deleting and recreating a budget does not forget what an earlier run charged against the same label. A scenario that exits part-way on a failed assertion skips its own cleanup, and the same-suffix rerun this file recommends for partial reruns would then meet an already-exhausted tiny budget and 429 on its first request — failing for a reason unrelated to what it tests, until someone cleared the state by hand. reset_release_budget resets a budget right after creation, so that first request behaves the same on the first run and the fifth. Confirmed by leaving an exhausted budget behind and rerunning S198 with the same suffix: exit 1 without the reset, exit 0 with it, same leftover state both times. The section preamble also claimed every scenario used tagging headers. S197 and S204 are admin-only and send no model traffic, and S200 relies on managed-key labels precisely because no tagging rule is configured, so it now says which scenarios do what. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both addressed. Failed scenarios retain budget state (P1) — confirmed and fixed. The key detail is that deleting and recreating a budget does not clear it: spend is a SUM over usage rows, so a fresh New Identical leftover state in both runs, so the reset is doing the work. Worth noting the same exposure exists in the pre-existing Preamble overclaims — correct, and it was wrong in both directions. Rewritten to say which scenarios configure tagging headers and restore the rule set (S198, S199, S201, S203), which uses managed-key labels instead (S200), and which are admin-only with no model traffic (S197, S204). Full set re-run after both changes: |
Adds S197–S204 to the release curl matrix (196 → 204), covering budgets scoped to a request label rather than a
user_pathsubtree, as shipped in #590.user_pathalongsidesubject, missing subject, unknown scope, delete-absent 404budget_exceeded, and a different-case spelling not blocked — label matching never folds caseuser_pathand alabelbudgetreset-oneclears a label budget's spendTwo assertions brought up to their prose
While verifying these against the implementation I found two scenarios whose descriptions promised more than the code checked. Both are now asserted:
Why S200 runs on the auth-enabled gateway
On a gateway with no
GOMODEL_MASTER_KEY, creating a managed key switches every endpoint —/v1/*included — to require bearer auth from then on, and managed keys have no delete endpoint (deactivatedoes not undo the switch, which counts stored keys rather than active ones). Registering one on the open SQLite gateway would leave it locked down for every later scenario. The auth-enabled gateway also has the exact response cache on, so both chat replies carry$QA_BUDGET_SUFFIX: a fixed reply string would be served from a prior run's cache on a rerun, bypassing usage tracking and leaving the budget permanently at zero spend.Verification
All eight pass against the live release stack on a fresh
QA_SUFFIX, so they are rerunnable and order-independent as documented:S199 and S201 were re-run after the assertion changes and still pass.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Tests