Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a manual budget reset API and store method, wires it through the HTTP server and handler, documents the new endpoint in OpenAPI, and adds coverage for success and error cases. ChangesManual Budget Reset
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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: 2
🤖 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 `@plugins/governance/budgetreset_test.go`:
- Around line 109-127: The idempotent reset test in
TestGovernanceStore_ResetBudget_Idempotent only checks the returned budget ID
and misses the main reset contract in the CAS-decline path. Update this test to
also assert the returned snapshot from ResetBudget has CurrentUsage reset to
zero (or the expected post-reset value) when ResetBudgetAt declines due to a
future LastReset, using buildBudgetWithUsage and ResetBudget as the key symbols
to locate the case.
In `@plugins/governance/store.go`:
- Around line 1867-1897: ResetBudget currently treats a failed ResetBudgetAt CAS
as success by returning the latest snapshot even when CurrentUsage may still be
non-zero. Update LocalGovernanceStore.ResetBudget so the fallback branch does
not silently succeed on a stale LastReset; either retry with a reset target that
is guaranteed to advance beyond the current timestamp or return an error when
the reset cannot be applied. Also update the idempotent test around ResetBudget
to assert that the returned budget from a manual reset has CurrentUsage equal to
zero.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 25716481-f3bd-472f-91a4-ccf88d342952
📒 Files selected for processing (7)
docs/openapi/openapi.jsonplugins/governance/budgetreset_test.goplugins/governance/store.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/governance_test.gotransports/bifrost-http/handlers/pricing_override_test.gotransports/bifrost-http/server/server.go
44564de to
493bff0
Compare
244a01d to
ce1b2a6
Compare
Summary
Adds a governance endpoint to manually reset a budget's usage on demand, without waiting for its reset window to elapse. Operators can now zero out a budget's
current_usageimmediately (e.g. after a billing correction or a mistaken spend) viaPOST /api/governance/budgets/{budget_id}/reset. The reset is applied to the in-memory governance store and persisted to the database, reusing the same side effects as an expiry-driven reset.Ref #4892
Changes
plugins/governance/store.go: addedResetBudget(ctx, budgetID)to theGovernanceStoreinterface and itsLocalGovernanceStoreimplementation. It advancesLastResetto now (restarting rolling windows; calendar-aligned budgets still reset at their next boundary), zeroes usage, zeroes theLastDBUsagesBudgetsbaseline so the next dump doesn't re-add stale usage, refreshes embedded VK budget references, fires the reset hook, and persists viaResetExpiredBudgets. Added theErrBudgetNotFoundsentinel error, returned for unknown IDs. A concurrent reset that loses the CAS returns the latest snapshot instead of erroring (idempotent).transports/bifrost-http/handlers/governance.go: addedResetBudgetto theGovernanceManagerinterface, registered thePOST /api/governance/budgets/{budget_id}/resetroute, and added theresetBudgethandler — 400 on invalid ID, 404 onErrBudgetNotFound, 500 on other errors, 200 with{ "budget": ... }on success.transports/bifrost-http/server/server.go: addedResetBudgettoServerCallbacksand implemented it onBifrostHTTPServer, delegating to the governance plugin's store.docs/openapi/openapi.json: documented the new endpoint (resetBudgetoperation,ManagementBearerAuth, 200/400/404/500 responses).plugins/governance/budgetreset_test.go(manual reset side effects, not-found, calendar-aligned, idempotent) and handler tests ingovernance_test.go(success, not-found). Updatedpricing_override_test.goto satisfy the extended interface.Design notes / trade-offs
ResetBudgetAt+ResetExpiredBudgetspath as expiry-driven resets, so persistence and hook behavior stay consistent.Type of change
Affected areas
How to test
Manual check against a running server:
Expected:
200with{ "budget": { ... "current_usage": 0, "last_reset": "<now>" ... } }. An unknownbudget_idreturns404; an empty/invalid ID returns400.Breaking changes
Related issues
Security considerations
The endpoint is protected by
ManagementBearerAuth, consistent with other governance management routes. Resetting a budget clears spend tracking, so access should remain restricted to management tokens.Checklist
docs/contributing/README.mdand followed the guidelines