feat: switch subscription plan#185
Conversation
Summary of ChangesHello @think-in-universe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the capability for users to change their subscription plans. It adds a new API endpoint and implements the underlying service logic to handle plan transitions, including crucial validation for agent instance limits when downgrading. The changes ensure that subscription updates are correctly processed through Stripe and reflected in the application's database, enhancing the flexibility of user subscriptions. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Review: feat: switch planCritical issues found before merge: 1. Privacy Violation (CLAUDE.md) — Logging Request Body at INFO LevelPer CLAUDE.md: "Never log request/response bodies". The plan name originates from user-submitted request body and is logged at INFO (production) level in two places: In
Remove these or downgrade to 2. Stripe/DB Inconsistency on Partial FailureThe Stripe subscription update happens outside the DB transaction. If Stripe succeeds but the DB upsert or commit fails, Stripe has the new plan but the DB still reflects the old one. The user could be billed at the new rate while the system enforces the old plan's limits (or vice versa): Consider reversing the operation order: update DB first (inside the transaction), then call Stripe, and if Stripe fails, roll back the DB transaction. Alternatively, rely on Stripe webhooks to reconcile state — but that path needs explicit documentation. 3. Missing proration_behavior in Stripe UpdateThe The expected billing behavior (immediate vs. end-of-period) should be a conscious decision, not a default. 4. TOCTOU Race Condition on Instance Limit CheckThe instance count is validated before the Stripe/DB update. A user could create additional agent instances between the check and when the plan change commits, bypassing the downgrade guard: At minimum, re-check the count inside the DB transaction (with a lock or re-query) to tighten the window. |
There was a problem hiding this comment.
Pull request overview
This PR implements a new subscription plan switching feature that allows users to change their subscription plan through the API.
Changes:
- Added
change_planmethod to subscription service that validates instance limits, updates Stripe subscription, and persists changes to database - Introduced
InstanceLimitExceedederror variant to prevent plan switches when user's instance count exceeds target plan's limit - Wired
AgentRepositorydependency into subscription service for instance count validation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/services/src/subscription/service.rs | Implements change_plan method with instance validation, Stripe API integration, and database transaction handling |
| crates/services/src/subscription/ports.rs | Adds InstanceLimitExceeded error variant and change_plan trait method signature |
| crates/api/src/routes/subscriptions.rs | Adds change_plan endpoint with request/response types and comprehensive error mapping |
| crates/api/src/openapi.rs | Registers change_plan endpoint and related schemas in OpenAPI documentation |
| crates/api/src/main.rs | Wires agent_repo into subscription service configuration |
| crates/api/tests/common.rs | Moves agent_repo initialization before subscription service (dependency requirement) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new change_plan functionality for subscriptions, enabling users to switch between different subscription plans. It integrates an agent_repo dependency into SubscriptionServiceConfig and SubscriptionServiceImpl for instance limit validation, and updates OpenAPI documentation and subscription routes. The review comments suggest improvements for code readability, organization, error handling, and consistency, including a positive note on the use of std::fmt::Display for enum string representation, which aligns with good practices. A comprehensive security audit found no vulnerabilities meeting medium, high, or critical severity criteria, and the implementation adheres to security best practices.
Fixes switch plan issue https://github.com/nearai/private-assistant/issues/44