feat(internal/account): billing service + webhook router#3
Merged
Conversation
Lands the v1 RPC business logic and the Stripe webhook event router.
Both packages depend on internal/shared/* (PR 2) and the migrations
from PR 1; both ship with full unit-test coverage using in-memory
fakes (no DB, no Stripe network calls in `go test`).
internal/account/billing/
types.go EnsureRequest/Response, PrepareAddPaymentMethod
Request/Response, GetPaymentMethodsRequest/Response,
PaymentMethod. Missing-array constants.
errors.go Typed Error with Code (INVALID_INPUT, NOT_FOUND,
STRIPE_ERROR, INTERNAL) + constructors. Wrapped
cause retained for log inspection; not serialized
over the wire.
store.go Store interface + pgxStore impl. EnsureAccount uses
a per-user advisory lock (pg_advisory_xact_lock on
hashtext('billing_account:user:'||user_id)) to
serialize SELECT-then-INSERT without a DB-level
unique constraint, matching the spec's
"handler-enforced" uniqueness.
service.go Three RPCs:
- Ensure: pure DB read; returns Missing array.
- PrepareAddPaymentMethod: idempotent account create
→ Stripe Customer create (lazy) → SetupIntent.
Orphan-Customer failure mode documented inline.
- GetPaymentMethods: mirror read, newest-first.
service_test.go 12 unit tests using fakeStore + fakeStripe.
internal/account/webhook/
router.go Status enum, Result struct, Store interface, Router
type. Process(): verify sig → idempotency insert →
dispatch by event type.
handlers.go Per-event handlers for the 5 v1 events:
- customer.created log only (we initiated)
- customer.updated touch updated_at + sync default_pm
- customer.deleted log only (erasure cascade is elsewhere)
- payment_method.attached insert mirror row
- payment_method.detached soft-delete mirror row
Non-card payment methods → unhandled (cards-only v1).
Drift cases (no accounts row for stripe customer) →
drift_warning status, 200 OK.
router_test.go 12 unit tests using fakeVerifier + fakeStore with
synthetic stripe-go event payloads.
go.mod: added jackc/pgx/v5 + pgxpool (for the pgxStore impl) and
promoted google/uuid (used by the public types) to direct dep.
Build status: go vet, go build, go test all green.
24 unit tests total across the two new packages.
Next PR (PR 4): cmd/account-api rewire with RPC envelope dispatch +
dual entry (Lambda + HTTP); cmd/account-webhook rewire to Router;
integration tests against Stripe test mode + testcontainers Postgres;
CI workflow update.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per design feedback: EnsureRequest now accepts a Require []string
field so callers can specify which capabilities to verify. Two
values supported in v1:
- "payment_method" — checks ms_billing.payment_methods_mirror for
a usable card (existing behavior).
- "subscription" — v1 stub: always returns "subscription" in
Missing because the ms_billing.subscriptions table doesn't yet
exist. Real check lands in v2 when the table ships.
Defaults preserve current behavior: empty Require → [payment_method].
Unknown Require values return INVALID_INPUT.
Per-capability checks run in deterministic order (payment_method
first, then subscription) so Missing's order is independent of the
request's Require ordering. A missing accounts row short-circuits
to ["billing_account"] alone — there's nothing else to usefully
check without an account.
Vocabulary now:
Request-side: RequirePaymentMethod, RequireSubscription
Response-side: MissingBillingAccount, MissingPaymentMethod, MissingSubscription
Tests added (5 new, 16 total in billing pkg):
- RequireSubscription returns ["subscription"] always
- Require both, PM met → ["subscription"]
- Require both, PM missing → ["payment_method", "subscription"]
(deterministic order)
- No accounts row + Require both → ["billing_account"]
- Unknown Require → INVALID_INPUT
Follow-up: mirrorstack-docs/api/billing/account-api.md needs an
update to document the Require field + the "subscription" value
in the Missing vocabulary. Separate docs PR after this lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aggregated findings from the three review agents (reuse, quality,
silent-failure). Applied 10 actionable items; skipped LOW stylistic +
out-of-scope (cmd/* wiring belongs to PR 4).
types.go
- New typed string `Capability` for EnsureRequest.Require entries.
Compile-time guards against typos at call sites; runtime validation
still catches arbitrary strings deserialized over the wire.
service.go
- Removed hand-rolled `contains([]string, string) bool` helper;
replaced with stdlib `slices.Contains` from Go 1.21+ (this module
targets Go 1.26).
- PrepareAddPaymentMethod: SetStripeCustomer failure now returns
billing.Internal (DB error) instead of billing.StripeError. The
failure isn't a Stripe API call; misclassifying it would lead
callers to the wrong retry path.
- PrepareAddPaymentMethod doc-comment renumbered + tightened.
service_test.go
- Require literal slices updated to []billing.Capability.
- New test: TestPrepareAddPaymentMethod_SetStripeCustomerFails_ReturnsInternal
locks in the error-code change.
store.go
- SetStripeCustomer now checks pgx.CommandTag.RowsAffected(); zero
rows returns ErrAccountNotFound instead of silently succeeding
(which would lead to a SetupIntent against an unanchored Customer).
- EnsureAccount advisory lock switched from 1-arg form
`pg_advisory_xact_lock(hashtext(...)::bigint)` to 2-arg
`pg_advisory_xact_lock(namespace, hashtext(user_id))`. The 1-arg
form risks birthday-paradox collisions at ~65K distinct users on
int32; the 2-arg form gives each per-user key its own int32 space
inside a fixed namespace prefix. Namespace constant
`advisoryLockNamespaceBillingAccountUser` documented inline.
errors.go
- Removed unused exported `NotFound()` constructor. `CodeNotFound`
constant kept (the dispatch layer can map future errors to it).
router.go
- dispatch switch cases now use stripe-go's typed event-type
constants (stripego.EventTypeCustomerCreated, etc.) instead of
raw string literals. Compile-time check against typos in the
Stripe API surface.
- NewRouter rejects nil verifier / store / log with explicit panics
instead of silently substituting slog.Default(). Wiring mistakes
surface at construction rather than as misconfigured production
logs.
handlers.go
- decodeCustomer + decodePaymentMethod now nil-guard event.Data
before dereferencing event.Data.Raw. stripe-go populates Data in
practice, but the guard prevents a panic on any malformed-but-
signature-valid payload.
- handlePaymentMethodDetached: added log on missing pm.ID guard and
on the SoftDeletePaymentMethod found=false no-op case. Monitoring
can now distinguish "expected idempotent retry" from "never-
mirrored PM" without inspecting DB state.
Build status: go vet, go build, go test all green. 17 unit tests in
billing pkg (was 16; +1 for SetStripeCustomer Internal), 12 in
webhook pkg. All pass.
Skipped (per finding triage):
- customer.updated drift idempotency: spec-design choice; reconciliation
is operational tooling, not handler concern.
- cmd/account-webhook/main.go Router wiring: PR 4 scope.
- stripe.NewClient empty-key fail-fast: PR 4 scope (touches
internal/shared/stripe, which is in PR 2 already merged).
- handleCustomerDeleted decode simplification, doc-comment numbering,
test json.Marshal error handling, card-expiry semantics comment:
LOW priority stylistic items.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
I-am-nothing
added a commit
that referenced
this pull request
Jun 16, 2026
Milestone D PR #3 — the ms_billing usage-metering storage + ingest spine. Metering is declaration-first: a module declares each metric once via ms.Meter(name, kind, ms.Unit, ms.Price); the declaration flows into the manifest and api-platform syncs it into metric_definitions via SetMetricDefinitions. kind is never on the wire (the catalog is authoritative) and an undeclared metric is rejected at ingest. Migrations 006-010 (born-clean, all ms_billing.*): - 006 metric_definitions + metric_kind enum (manifest-fed catalog; unit_price_micros = final per-unit customer price, NULL = unpriced) - 007 usage_events (raw, immutable, idempotent on event_id; lazy NULL account; declared kind snapshotted) - 008 billing_periods (table only; its writer ships with the cycle binary) - 009 usage_aggregates (snapshot billable record; 10/10 = NO markup for custom metrics; margin_share_class is developer-settlement only) - 010 module_visibility (developer margin-share mirror; default private) internal/account/usage (new pkg) — Store + pgxStore + in-memory fake + Service: - RecordUsage: idempotent ON CONFLICT(event_id) DO NOTHING; resolves the declared kind from the catalog and REJECTS an undeclared or retired metric (INVALID_INPUT); rejects reserved platform.* / infra.*; lazy owner-account resolution. - GetUsageSummary: live current-period charged per metric = quantity × declared unit_price (NO blanket 1.2×; custom metric = declared price). - SetMetricDefinitions: validates the full set, then upserts it in ONE transaction (all-or-nothing) so the catalog is never partially synced. - SetModuleVisibility: upserts the developer margin-share mirror. cmd/account-api — dispatcher cases + chi routes; RecordUsage gated by a dedicated meter secret/header (METER_SECRET / X-MS-Meter-Secret), never overloading X-MS-Internal-Secret. Deferred to PR #5/#6: period rollup + per-kind aggregation, the flat-1.2× platform-infra markup, OpenPeriodForAccount, Stripe charge + cycle binary + migrations 011-013. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
I-am-nothing
added a commit
that referenced
this pull request
Jun 17, 2026
feat: usage metering storage + ingest (Milestone D PR #3)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Lands the v1 RPC business logic and the Stripe webhook event router. Two new packages; full unit-test coverage with in-memory fakes (no DB, no Stripe network).
internal/account/billing/— RPC handlerstypes.gomirrorstack-docs/api/billing/account-api.md;MissingBillingAccount/MissingPaymentMethodconstants.errors.go*ErrorwithCode(INVALID_INPUT, NOT_FOUND, STRIPE_ERROR, INTERNAL) + constructors. Wrapped cause retained for logs; not serialized.store.goStoreinterface +pgxStore.EnsureAccountuses a per-user advisory lock (pg_advisory_xact_lockon `hashtext('billing_account:user:'service.goEnsure(pure DB read),PrepareAddPaymentMethod(idempotent account create → lazy Stripe Customer → SetupIntent — orphan failure mode documented inline),GetPaymentMethods(mirror read, newest-first).service_test.gofakeStore+fakeStripe.internal/account/webhook/— Stripe webhook routerrouter.goStatusenum,Result,Storeinterface,Router.Process(): verify sig → idempotency insert → dispatch by event type.handlers.gounhandled. Drift cases (no accounts row for Stripe customer) ackdrift_warning+ 200.router_test.gofakeVerifier+fakeStoredriving synthetic Stripe event payloads.go.mod
Added direct deps:
github.com/jackc/pgx/v5+pgxpool(forpgxStore).github.com/google/uuid(promoted to direct; was indirect via stripe-go).Coverage
24 unit tests across the two packages. Builds clean, vets clean, all tests pass.
Integration tests against real Postgres + Stripe test mode land in PR 4 alongside the cmd/ rewiring (need testcontainers + GitHub Secret plumbing).
Notable design decisions baked in
Per-user advisory lock for EnsureAccount rather than adding a
(owner_kind, owner_user_id)partial unique constraint. The schema doc says "handler-enforced" uniqueness; the advisory lock honors that while still being correct under concurrent calls.Drift cases ack with 200 rather than 5xx. A
payment_method.attachedevent for a customer with no accounts row is a Stripe→DB inconsistency, not a code bug. Returning 5xx would cause Stripe to retry the same broken event indefinitely. The handler logs + returns{"status": "drift_warning"}so monitoring can catch it.Non-card PMs are explicitly unhandled (no error). v1 supports cards only per the spec; SEPA / Link / etc. arrive as
payment_method.attachedevents with nocardblock, and the handler acks withunhandledrather than erroring.is_defaultis sourced fromcustomer.updated, not frompayment_method.attached. This keeps webhook processing to a single DB write per event (no synchronous Stripe API call inside the handler transaction). The follow-oncustomer.updatedevent auto-fires when Stripe sets a default PM.Idempotency-insert FIRST in
Process(). If the side-effect step errors, the whole transaction rolls back including the idempotency record. Stripe retries → next attempt re-enters the side-effect path cleanly.PR sequence
feat(migrations): 002 + 003483ac87)feat(internal/shared): middleware + Stripe wrapperd0498ea)feat(internal/account): billing service + webhook routerfeat(cmd): rewire account-api + account-webhook; integration tests; CI for Stripe test mode🤖 Generated with Claude Code