feat: add budget spending limits to api-keys service

[bas4r]

Summary

• Add spending limits module to the api-keys Rust service with daily, weekly, monthly, and annual time windows
• gRPC server endpoints for checking limits and recording spending per API key
• PostgreSQL migration for spending_limits table
• GraphQL mutations for setting and removing limits on each time window
• Updated subgraph schema with ApiKeyLimits type and limit mutation responses
• Updated supergraph schema (Apollo Federation, API_KEYS graph only)
• Oathkeeper config to pass api_key_id in JWT claims
• Bats GQL fixtures for all limit set/remove mutations
• Minor jq fix in existing api-keys.bats

Part 1 of 2 — This PR contains the api-keys service changes only.
Part 2 (core API integration + dashboard UI) will be stacked on top.

Test plan

• Verify api-keys service builds: buck2 build //core/api-keys:api-keys
• Verify supergraph schema compiles: buck2 test //dev:check-sdls
• Verify bats GQL fixtures are valid

[dolcalmi]

I guess you are missing docker compose config updates and run sqlx prepare cmd

[Copilot]

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[grimen]

@blink-claw-bot review

[blink-claw-bot]

Good work overall — the architecture is clean: separate Limits struct with DB layer, gRPC server, GraphQL mutations with proper auth checks, and a sensible migration.

A few things beyond what dolcalmi already covered:

GraphQL type mismatch (important): The ApiKeyLimits struct exposes i64 fields, but the GraphQL schema maps Int to 32-bit. Copilot flagged this too — values above ~2.1B sats (~21 BTC) will overflow on the GraphQL side. Consider using BigInt / SatAmount scalar for the limit fields, consistent with how sats are handled elsewhere in the codebase.

On dolcalmi's enum suggestion (LimitTimeWindow enum + single mutation): +1, this is the right call. The current approach with LimitTimeWindow enum and apiKeySetLimit / apiKeyRemoveLimit is already doing this — the code looks good as-is. The find_owned_api_key helper is a nice dedup. One small thing: the helper fetches ALL keys for the subject just to find one — could use a direct DB query by (subject_id, api_key_id) to avoid loading the full list, but fine for now since key counts per user are small.

Race condition in check_spending_limit + record_spending: These are separate calls with no atomicity guarantee. Between check and record, another request could also pass the check. For Part 1 this is probably acceptable (the gRPC caller in Part 2 will need to handle this), but worth noting for Part 2 — consider a single check_and_record atomic operation or use SELECT ... FOR UPDATE.

get_all_spending — single query is correct: I see the final version consolidates spending into one query with FILTER clauses. This addresses dolcalmi's earlier comment about multiple queries. Good.

gRPC remaining_*_sats computation is duplicated: The remaining_* calculation (limit - spent, clamped to 0) is copy-pasted between check_spending_limit and get_spending_summary handlers. Extract to a helper on the summary struct.

Minor: use tracing::{grpc, instrument}; in grpc/server/mod.rs — is tracing::grpc a crate-local extension? Standard tracing doesn't have a grpc module. Just want to confirm this compiles.

Overall: solid foundation for spending limits. The main concern is the i32/i64 type mismatch on the GraphQL layer — that should be fixed before merge. The rest is cleanup.

🤖 Review by claude-opus-4 via Blink-Claw-Bot

[blink-claw-bot]

Thanks for the quick fixes and the clear explanation on the Int type — you're right, GraphQL's 32-bit Int acts as a natural cap before the i64 Rust code, so the mismatch is harmless in practice. Makes sense.

LGTM on the changes I flagged. The enum-based approach for window types is a nice improvement over the original struct design.

🤖 Follow-up by claude-opus-4 via Blink-Claw-Bot

[grimen]

From what I gather test coverage for the diff is 5-10% - ensure you add complete test coverage. Both unit and integration/e2e, and to clarify tests should cover edge cases, and concurrent calls since the nature if this implementation has potential race condition.

[dolcalmi]

From what I gather test coverage for the diff is 5-10% - ensure you add complete test coverage. Both unit and integration/e2e, and to clarify tests should cover edge cases, and concurrent calls since the nature if this implementation has potential race condition.

some comments:

• please use e2e over integration tests (only add/use them if they are necessary)
• add unit tests in api keys project not in api/core

[bas4r]

I guess you are missing docker compose config updates and run sqlx prepare cmd

cargo sqlx prepare was failing due to some dependency version mismatch on cargo.lock. Build was passing on buck2 but apparently buck2 uses /third-party/rust/Cargo.lock which has the correct dependency versions for the rust version 1.77.2 we have on the nix environment. So initially i was using sqlx::query() instead of sqlx::query!() macro that requires the offline query files.

I bypassed the error upgrading the root Cargo.lock (the one nix environment uses with cargo commands) to match failing dependencies with the buck2. And refactored all new queries to sqlx::query!() and committed the new sqlx prepare query files. It's consistent with the rest of the project now.

Also added the grpc port on docker compose api-key config.

All review comments has been addressed.