feat: add Stripe-based billing subscription management #23

solimander · 2025-10-27T18:25:40Z

Summary

Add eigenx billing commands for Stripe-based subscription management
Add billing API client with environment-specific auth
Enforce quota checks during app deployment

Changes

New commands: billing subscribe, billing cancel, billing status
Deploy now checks available quota before proceeding
Browser-based Stripe checkout flow for subscriptions

claude · 2025-10-27T18:25:58Z

Claude finished @solimander's task —— View job

PR Review: Stripe-based Billing Subscription Management

Todo List:

Read repository conventions from CLAUDE.md
Review billing command structure and implementation
Analyze security considerations for Stripe integration
Check error handling and user experience
Evaluate code quality and best practices
Look for potential performance issues
Assess test coverage needs
Provide comprehensive feedback

Code Review Summary

This PR successfully adds Stripe-based billing subscription management to the eigenx CLI. The implementation is well-structured and follows Go best practices. Here's my detailed analysis:

✅ Strengths

Clean Architecture: Commands are properly organized under pkg/commands/billing/ following the existing pattern
Security-First Design: Uses cryptographic authentication with KMS keys for API calls
User Experience: Good progressive disclosure with helpful error messages and billing portal links
Environment Awareness: Properly integrates with existing environment configuration system
Quota Integration: Deployment process correctly checks billing status before proceeding

🔍 Code Quality & Best Practices

Good Practices:

Consistent error handling with wrapped errors (fmt.Errorf)
Proper context propagation throughout the call chain
Good separation of concerns between commands and API client
Follows existing CLI patterns with urfave/cli/v2

Suggestions for Improvement:

pkg/commands/billing/subscribe.go:73-80

The 5-minute timeout for payment polling is reasonable, but consider adding a progress indicator:

logger.Info("Waiting for payment completion... (timeout in 5 minutes)")

pkg/commands/billing/status.go:87-91

The price display logic could be more robust:

if subscription.PlanPrice != nil && subscription.Currency != nil && *subscription.PlanPrice > 0 {
    logger.Info("Plan: $%.2f/month", *subscription.PlanPrice) 
} else {
    logger.Info("Plan: Standard")
}

Consider handling different currencies and showing the currency symbol.

🔒 Security Analysis

Excellent Security Practices:

All billing API calls require CanManageBillingPermission
Cryptographic authentication using KMS keys
No sensitive data (API keys, tokens) stored locally
Browser-based checkout prevents CLI from handling payment details
Proper permission-based access control

No security issues found - the implementation follows security best practices throughout.

🚀 Performance Considerations

Good Performance Characteristics:

30-second HTTP client timeout prevents hanging
Efficient polling with 3-second intervals
Proper error handling prevents unnecessary retries
Contract calls are minimal and cached where appropriate

🧪 Test Coverage Recommendations

The implementation would benefit from:

Unit tests for status formatting:

func TestFormatStatus(t *testing.T) {
    assert.Equal(t, "✓ Active", formatStatus(utils.StatusActive))
    assert.Equal(t, "⚠ Past Due", formatStatus(utils.StatusPastDue))
}

Integration tests for quota checking:
- Test deployment blocking when quota exceeded
- Test subscription activation enabling deployment
Error handling tests:
- API failures, network timeouts
- Invalid subscription states

🐛 Potential Issues

Minor Issues:

pkg/commands/billing/status.go:102-104: The chained error handling could be cleaner:

if developerAddr, err := utils.GetDeveloperAddress(cCtx); err != nil {
    logger.Warn("  Unable to fetch usage: failed to get developer address")
} else if count, err := caller.GetActiveAppCount(cCtx.Context, developerAddr); err != nil {
    logger.Warn("  Unable to fetch usage: %v", err)  
} else {
    logger.Info("  %d / 1 apps deployed on %s", count, envName)
}

pkg/commands/billing/cancel.go:121-139: The getActiveAppsByCreator function fetches up to 1,000 apps which could be slow for users with many apps. Consider pagination or filtering at the contract level.

✨ User Experience Excellence

Outstanding UX Features:

Clear status indicators with emojis (✓, ⚠, ✗)
Helpful guidance for next steps in each state
Graceful handling of payment issues with portal links
App suspension warnings before cancellation
Browser auto-opening with fallback URL display

📋 Integration Quality

The billing integration seamlessly fits into the existing codebase:

Follows established patterns in pkg/commands/app/
Reuses existing utilities and configurations
Properly integrates with contract caller system
Maintains consistency with other commands

Overall Assessment

This is a high-quality implementation that successfully adds billing functionality while maintaining the CLI's existing patterns and security standards. The code is production-ready with excellent error handling and user experience.

Recommended Actions:

Add unit tests for status formatting and error scenarios
Consider adding integration tests for quota enforcement
Minor code cleanup for chained error handling

The implementation demonstrates strong engineering practices and security awareness. Great work! 🎉

pkg/common/browser.go

gpsanant · 2025-10-27T19:53:23Z

pkg/commands/billing/subscribe.go

+
+		// Poll for subscription activation
+		logger.Info("\nWaiting for payment completion...")
+		timeout := time.After(5 * time.Minute)


why timeout? just allow them to ctrl+C

didn't seem worth continuing to poll after 5 mins or so. they can also view the billing status via eigenx billing status if it stops watching

cool. will check when i go through it myself

gpsanant · 2025-10-27T20:01:29Z

pkg/commands/billing/cancel.go

+
+		for env, info := range results {
+			if info.Count > 0 {
+				networkAppCounts[env] = info.Count


dont u already have this in results?

gpsanant · 2025-10-30T07:27:46Z

pkg/commands/billing/status.go

+
+		// Plan details
+		if subscription.PlanPrice != nil && subscription.Currency != nil && *subscription.PlanPrice > 0 {
+			logger.Info("Plan: $%.2f %s", *subscription.PlanPrice, strings.ToUpper(*subscription.Currency))


need $ and currency?

gpsanant · 2025-10-30T07:28:55Z

pkg/commands/billing/status.go

+
+		// Cancellation status
+		if subscription.CancelAtPeriodEnd != nil && *subscription.CancelAtPeriodEnd {
+			if subscription.CurrentPeriodEnd != nil {


another &&?

gpsanant · 2025-10-30T07:29:13Z

pkg/commands/billing/status.go

+		} else {
+			logger.Info("  %d / 1 apps deployed on %s", count, envName)
+			logger.Info("")
+		}


just split up into if err != nils?

if/else feels cleaner given we pass-through imo

…h environment-specific auth

…ointer dereference

…d subscriptions

…add portal URL display

- Refactored subscription management to handle subscriptions per environment instead of globally - Updated status command to show subscription details and app usage for current environment only - Modified cancel command to suspend apps only on current environment when canceling - Added improved handling of different subscription states (past_due, unpaid, etc.) with portal links - Enhanced status display formatting with clearer subscription

solimander changed the title ~~feat: add Stripe-based billing subscription management~~ feat: add Stripe-based billing subscription management Oct 27, 2025

gpsanant reviewed Oct 27, 2025

View reviewed changes

pkg/common/browser.go Outdated Show resolved Hide resolved

gpsanant reviewed Oct 27, 2025

View reviewed changes

solimander marked this pull request as ready for review October 28, 2025 14:46

solimander force-pushed the soli/billing-mmvp branch from 4b9e50a to 3927170 Compare October 30, 2025 03:08

gpsanant reviewed Oct 30, 2025

View reviewed changes

solimander added 13 commits November 4, 2025 12:55

feat: add billing commands and quota checks for app deployment

db9a966

refactor: use dedicated billing API client for billing operations wit…

f8d0247

…h environment-specific auth

fix: handle nullable fields in subscription response to prevent nil p…

df87d6b

…ointer dereference

feat: display cancellation details and resubscribe prompt for cancele…

e2318e1

…d subscriptions

fix: prevent displaying next charge when upcoming invoice date is zero

e7b2506

refactor: replace custom browser opener with pkg/browser library and …

04b7912

…add portal URL display

full suspension support

6fef4b1

feat: improve billing status display format

f648fd4

add environment flag to billing commands

7d3fd84

remove "USD"

e0daaf7

improve billing status code organization and readability

0a04e8b

fix: filename spelling

0f1a7a5

solimander force-pushed the soli/billing-mmvp branch from 97f7307 to 0f1a7a5 Compare November 4, 2025 20:55

gpsanant approved these changes Nov 4, 2025

View reviewed changes

solimander merged commit 1cb3c80 into main Nov 4, 2025
12 checks passed

solimander deleted the soli/billing-mmvp branch November 4, 2025 21:08

feat: add Stripe-based billing subscription management #23

feat: add Stripe-based billing subscription management #23

Uh oh!

Conversation

solimander commented Oct 27, 2025

Summary

Changes

Uh oh!

claude bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Stripe-based Billing Subscription Management

Code Review Summary

✅ Strengths

🔍 Code Quality & Best Practices

pkg/commands/billing/subscribe.go:73-80

pkg/commands/billing/status.go:87-91

🔒 Security Analysis

🚀 Performance Considerations

🧪 Test Coverage Recommendations

🐛 Potential Issues

Minor Issues:

✨ User Experience Excellence

📋 Integration Quality

Overall Assessment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

solimander Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude bot commented Oct 27, 2025 •

edited

Loading

solimander Oct 27, 2025 •

edited

Loading