Fix Phase 16 code review issues

[menvil]

Fixes 6 code review violations from Phase 16

P1 — Race condition in monthly credit grant (SubscriptionWebhookHandler.php:38)

Root cause: alreadyGrantedForInvoice() check and grant() call were not atomic — two concurrent webhook workers could both pass the check before either wrote.
Fix: Wrapped handleInvoicePaid body in DB::transaction() with lockForUpdate() on the credit account row. Under MySQL/Postgres this serializes concurrent requests for the same user via row-level locking. Under SQLite (tests) transactions are already serialized at the connection level.

P2 — Cross-provider collision in webhook event unique index

Root cause: billing_webhook_events.provider_event_id was globally unique, so identical event IDs from two different payment providers would silently drop one.
Fix: New migration drops the single-column unique index and replaces it with a composite (provider, provider_event_id) unique. BillingWebhookEventRecorder::recordIfNew() and wasProcessed() updated to use both columns.

P2 — Billing views render as unstyled HTML

Root cause: Both views were created as plain HTML stubs to unblock route registration in CONV-245.
Fix: Both views now extend <x-layouts.app> with title, navigation, app styling, and a "Go to dashboard" link.

P2 — Config fallbacks mask missing Stripe price IDs

Root cause: env('STRIPE_PRO_PRICE_ID', 'price_test_pro') silently returns a fake ID in any environment where the var isn't set.
Fix: Removed string fallbacks — env() now returns null when the var is absent. Test price IDs moved to phpunit.xml so tests remain green. BillingPaymentService already blocks on blank() price IDs before calling Stripe.

P2 — Empty string Stripe price IDs slip through guard

Root cause: $plan->stripePriceId === null does not catch ''.
Fix: Changed to blank($plan->stripePriceId) which catches null, '', and ' '.

Test plan

• composer test passes (459 tests)
• composer lint passes
• npm run build passes

🤖 Generated with Claude Code

---

Summary by cubic

Fix billing race conditions and webhook uniqueness, improve billing views, and tighten Stripe price ID handling to prevent invalid checkouts.

• Bug Fixes

  • Prevent double monthly credit grants by making invoice webhook handling atomic with a DB transaction and row-level lock.
  • Avoid cross-provider collisions for webhook event IDs.
  • Reject blank Stripe price IDs in paid checkout.
  • Render billing success/cancel pages with the app layout, styling, and a dashboard link.

• Migration

  • Run the migration to change the billing_webhook_events unique index to composite (provider, provider_event_id).
  • Ensure STRIPE_PRO_PRICE_ID and STRIPE_MAX_PRICE_ID are set in environment variables (defaults were removed).

^{Written for commit fe467be. Summary will update on new commits.}

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

• release

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 41c121fd-d5e5-407d-bd1a-306ee63b4628

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/phase16-code-review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}