fix(billing): support invoice delete source handling (#4554)

Author: turip

.agents/skills/billing/SKILL.md

@@ -174,6 +174,8 @@ DeleteInProgress → DeleteSyncing → Deleted (TriggerFailed → DeleteFailed)
 
 **Retrying stuck invoices**: Use the existing `RetryInvoice` service method (`service/invoice.go`) rather than firing `TriggerRetry` directly. `RetryInvoice` first **downgrades all critical validation issues to warnings** before firing the trigger — without this step, `noCriticalValidationErrors` would immediately block re-advancement out of `DraftValidating` and the invoice would land back in `DraftSyncFailed`. For bulk retries, query with `ExtendedStatuses: []billing.StandardInvoiceStatus{billing.StandardInvoiceStatusDraftSyncFailed}` and call `RetryInvoice` per result.
 
+**Delete lifecycle semantics**: deleting an invoice is modeled as a delete action, not a generic retry. `DeleteInvoice` fires `TriggerDelete` with a typed delete trigger input that carries the delete source. `DeleteInProgress` consumes that input in an `OnEntry` action, stamps the source onto the invoice for audit, and performs source-specific cleanup. If delete syncing fails and the invoice reaches `DeleteFailed`, calling `DeleteInvoice` again should fire `TriggerDelete` again; do not route delete failures through `RetryInvoice`.
+
 ## Line Engine Lifecycle
 
 The billing line-engine contract lives in `openmeter/billing/lineengine.go`. Billing owns the orchestration and grouping; each engine owns only the behavior for the lines assigned to its discriminator.

.agents/skills/charges/SKILL.md

@@ -153,6 +153,7 @@ Important rules:
 - usage-based payment handling is intentionally different from flat-fee and credit-purchase: the usage-based state machine owns realization only, while the usage-based line engine/run service records payment authorization/settlement directly on historical runs and only re-enters the state machine through an aggregate trigger (for example `all_payments_settled`) once all invoiced runs on the charge are settled. Do not apply this rule generically to flat-fee or credit-purchase; those charge types may still keep payment states inside their own state machines.
 - usage-based invoice branches should read in this order: `started -> waiting_for_collection -> processing -> issuing -> completed`, then auto-advance out of the branch. Keep `invoice_issued` as the boundary between `processing` and `issuing`, run `FinalizeInvoiceRun(...)` from the `issuing` state, and let `completed` be the last branch-local status before `next` returns a partial invoice to `active` or moves a final invoice to `active.awaiting_payment_settlement`
 - when adding or renaming usage-based detailed statuses, remember that `status_detailed` is an Ent enum for `ChargeUsageBased`; run `make generate` so the generated enum validators and migrate schema include the new values before trusting state-machine changes
+- when charge code deletes an empty standard invoice, call billing `DeleteInvoice` with `billing.ChangeSourceSystem`; billing stores that source for audit and dispatches `OnMutableStandardLinesDeletedBySystem(...)` only for non-deleted standard lines so charge engines can clean up line-backed runs without mutating the deleted invoice's visible line history
 
 Flat-fee credit-then-invoice lifecycle rules:
 

openmeter/billing/adapter/invoice.go

@@ -482,6 +482,7 @@ func (a *adapter) UpdateStandardInvoice(ctx context.Context, in billing.UpdateSt
 			SetOrClearDraftUntil(convert.SafeToUTC(in.DraftUntil)).
 			SetOrClearIssuedAt(convert.SafeToUTC(in.IssuedAt)).
 			SetOrClearDeletedAt(convert.SafeToUTC(in.DeletedAt)).
+			SetOrClearDeletionSource(lo.EmptyableToPtr(in.DeletionSource)).
 			SetOrClearSentToCustomerAt(convert.SafeToUTC(in.SentToCustomerAt)).
 			SetOrClearQuantitySnapshotedAt(convert.SafeToUTC(in.QuantitySnapshotedAt))
 
@@ -688,11 +689,12 @@ func (a *adapter) mapStandardInvoiceBaseFromDB(invoice *db.BillingInvoice) billi
 			},
 			UsageAttribution: mapCustomerUsageAttributionFromDB(invoice.CustomerID, invoice.CustomerKey, invoice.CustomerUsageAttribution),
 		},
-		Period:    mapPeriodFromDB(invoice.PeriodStart, invoice.PeriodEnd),
-		IssuedAt:  convert.TimePtrIn(invoice.IssuedAt, time.UTC),
-		CreatedAt: invoice.CreatedAt.In(time.UTC),
-		UpdatedAt: invoice.UpdatedAt.In(time.UTC),
-		DeletedAt: convert.TimePtrIn(invoice.DeletedAt, time.UTC),
+		Period:         mapPeriodFromDB(invoice.PeriodStart, invoice.PeriodEnd),
+		IssuedAt:       convert.TimePtrIn(invoice.IssuedAt, time.UTC),
+		CreatedAt:      invoice.CreatedAt.In(time.UTC),
+		UpdatedAt:      invoice.UpdatedAt.In(time.UTC),
+		DeletedAt:      convert.TimePtrIn(invoice.DeletedAt, time.UTC),
+		DeletionSource: lo.FromPtr(invoice.DeletionSource),
 
 		CollectionAt:               normalizeOptionalTime(invoice.CollectionAt),
 		PaymentProcessingEnteredAt: convert.TimePtrIn(invoice.PaymentProcessingEnteredAt, time.UTC),

openmeter/billing/charges/invoiceupdater/invoiceupdate.go

@@ -456,7 +456,10 @@ func (u *Updater) updateMutableStandardInvoice(ctx context.Context, invoice bill
 			return nil
 		}
 
-		invoice, err := u.billingService.DeleteInvoice(ctx, updatedInvoice.GetInvoiceID())
+		invoice, err := u.billingService.DeleteInvoice(ctx, billing.DeleteInvoiceInput{
+			Invoice:        updatedInvoice.GetInvoiceID(),
+			DeletionSource: billing.ChangeSourceSystem,
+		})
 		if err != nil {
 			return fmt.Errorf("deleting empty invoice: %w", err)
 		}

openmeter/billing/httpdriver/invoice.go

@@ -354,9 +354,12 @@ func (h *handler) DeleteInvoice() DeleteInvoiceHandler {
 				return DeleteInvoiceRequest{}, fmt.Errorf("failed to resolve namespace: %w", err)
 			}
 
-			return billing.InvoiceID{
-				ID:        params.InvoiceID,
-				Namespace: ns,
+			return billing.DeleteInvoiceInput{
+				Invoice: billing.InvoiceID{
+					ID:        params.InvoiceID,
+					Namespace: ns,
+				},
+				DeletionSource: billing.ChangeSourceAPIRequest,
 			}, nil
 		},
 		func(ctx context.Context, request DeleteInvoiceRequest) (DeleteInvoiceResponse, error) {

openmeter/billing/service/invoice.go

@@ -595,6 +595,7 @@ type executeTriggerOnInvoiceOptions struct {
 	editCallback        func(sm *InvoiceStateMachine) error
 	allowInStates       []billing.StandardInvoiceStatus
 	includeDeletedLines bool
+	triggerArgs         []any
 }
 
 func ExecuteTriggerWithEditCallback(cb editCallbackFunc) executeTriggerApplyOptionFunc {
@@ -615,6 +616,12 @@ func ExecuteTriggerWithIncludeDeletedLines(includeDeletedLines bool) executeTrig
 	}
 }
 
+func ExecuteTriggerWithArgs(args ...any) executeTriggerApplyOptionFunc {
+	return func(opts *executeTriggerOnInvoiceOptions) {
+		opts.triggerArgs = args
+	}
+}
+
 func (s *Service) executeTriggerOnInvoice(ctx context.Context, invoiceID billing.InvoiceID, trigger billing.InvoiceTrigger, opts ...executeTriggerApplyOptionFunc) (billing.StandardInvoice, error) {
 	if err := invoiceID.Validate(); err != nil {
 		return billing.StandardInvoice{}, billing.ValidationError{
@@ -632,7 +639,7 @@ func (s *Service) executeTriggerOnInvoice(ctx context.Context, invoiceID billing
 			InvoiceID:           invoiceID,
 			IncludeDeletedLines: options.includeDeletedLines,
 			Callback: func(ctx context.Context, sm *InvoiceStateMachine) error {
-				canFire, err := sm.CanFire(ctx, trigger)
+				canFire, err := sm.CanFire(ctx, trigger, options.triggerArgs...)
 				if err != nil {
 					return fmt.Errorf("checking if can fire: %w", err)
 				}
@@ -670,7 +677,7 @@ func (s *Service) executeTriggerOnInvoice(ctx context.Context, invoiceID billing
 					}
 				}
 
-				if err := sm.FireAndActivate(ctx, trigger); err != nil {
+				if err := sm.FireAndActivate(ctx, trigger, options.triggerArgs...); err != nil {
 					validationIssues, err := billing.ToValidationIssues(err)
 					sm.Invoice.ValidationIssues = validationIssues
 
@@ -714,18 +721,25 @@ func (s *Service) DeleteInvoice(ctx context.Context, input billing.DeleteInvoice
 	}
 
 	// Let's see if we are talking about a gathering invoice
-	invoiceType, err := s.adapter.GetInvoiceType(ctx, input)
+	invoiceType, err := s.adapter.GetInvoiceType(ctx, input.Invoice)
 	if err != nil {
 		return billing.StandardInvoice{}, fmt.Errorf("getting invoice type: %w", err)
 	}
 
 	if invoiceType == billing.InvoiceTypeGathering {
 		return billing.StandardInvoice{}, billing.ValidationError{
-			Err: fmt.Errorf("gathering invoice[%s]: %w", input.ID, billing.ErrInvoiceCannotDeleteGathering),
+			Err: fmt.Errorf("gathering invoice[%s]: %w", input.Invoice.ID, billing.ErrInvoiceCannotDeleteGathering),
 		}
 	}
 
-	return s.executeTriggerOnInvoice(ctx, input, billing.TriggerDelete)
+	return s.executeTriggerOnInvoice(
+		ctx,
+		input.Invoice,
+		billing.TriggerDelete,
+		ExecuteTriggerWithArgs(billing.DeleteInvoiceTriggerInput{
+			Source: input.DeletionSource,
+		}),
+	)
 }
 
 // updateInvoice calls the adapter to update the invoice and returns the updated invoice including any expands that are

openmeter/billing/service/invoiceupdate.go

@@ -493,12 +493,12 @@ func validateLineEngineResult(expectedLines []billing.GenericInvoiceLine, actual
 	return errors.Join(errs...)
 }
 
-func (s *Service) dispatchSystemStandardLineDeletions(ctx context.Context, invoice billing.StandardInvoice, lineDiff mutableInvoiceLineDiff) error {
-	if len(lineDiff.Deleted) == 0 {
+func (s *Service) dispatchSystemStandardLineDeletions(ctx context.Context, invoice billing.StandardInvoice, deletedLinesIn []billing.GenericInvoiceLine) error {
+	if len(deletedLinesIn) == 0 {
 		return nil
 	}
 
-	deletedLines, err := slicesx.MapWithErr(lineDiff.Deleted, func(line billing.GenericInvoiceLine) (*billing.StandardLine, error) {
+	deletedLines, err := slicesx.MapWithErr(deletedLinesIn, func(line billing.GenericInvoiceLine) (*billing.StandardLine, error) {
 		standardLine, err := line.AsInvoiceLine().AsStandardLine()
 		if err != nil {
 			return nil, fmt.Errorf("converting deleted line[%s] to standard line: %w", line.GetID(), err)

openmeter/billing/service/lineengine_test.go

@@ -41,13 +41,9 @@ func TestDispatchSystemStandardLineDeletionsGroupsLinesByEngine(t *testing.T) {
 	invoiceLine := newStandardLineForLineEngineTest("line-1", billing.LineEngineTypeInvoice, true)
 	chargeLine := newStandardLineForLineEngineTest("line-2", billing.LineEngineTypeChargeUsageBased, true)
 
-	require.NoError(t, svc.dispatchSystemStandardLineDeletions(t.Context(), invoice, mutableInvoiceLineDiff{
-		OnMutableInvoiceUpdateInput: billing.OnMutableInvoiceUpdateInput{
-			Deleted: []billing.GenericInvoiceLine{
-				invoiceLine.AsGenericLine(),
-				chargeLine.AsGenericLine(),
-			},
-		},
+	require.NoError(t, svc.dispatchSystemStandardLineDeletions(t.Context(), invoice, []billing.GenericInvoiceLine{
+		invoiceLine.AsGenericLine(),
+		chargeLine.AsGenericLine(),
 	}))
 
 	require.Len(t, invoiceEngine.deletedBySystemInputs, 1)
@@ -81,12 +77,8 @@ func TestDispatchSystemStandardLineDeletionsReturnsEngineError(t *testing.T) {
 		},
 	}
 
-	err := svc.dispatchSystemStandardLineDeletions(t.Context(), invoice, mutableInvoiceLineDiff{
-		OnMutableInvoiceUpdateInput: billing.OnMutableInvoiceUpdateInput{
-			Deleted: []billing.GenericInvoiceLine{
-				newStandardLineForLineEngineTest("line-1", billing.LineEngineTypeInvoice, true).AsGenericLine(),
-			},
-		},
+	err := svc.dispatchSystemStandardLineDeletions(t.Context(), invoice, []billing.GenericInvoiceLine{
+		newStandardLineForLineEngineTest("line-1", billing.LineEngineTypeInvoice, true).AsGenericLine(),
 	})
 
 	require.ErrorIs(t, err, errEngineFailed)
@@ -168,6 +160,79 @@ func TestOnUnsupportedCreditNoteReturnsEngineError(t *testing.T) {
 	require.ErrorIs(t, err, errEngineFailed)
 }
 
+func TestDeleteInvoiceSystemDeletionSourceDispatchesOnlyNonDeletedLines(t *testing.T) {
+	invoiceEngine := &recordingLineEngine{
+		NoopLineEngine: billingtestutils.NoopLineEngine{
+			EngineType: billing.LineEngineTypeInvoice,
+		},
+	}
+
+	svc := &Service{
+		lineEngines: newEngineRegistry(),
+	}
+	require.NoError(t, svc.RegisterLineEngine(invoiceEngine))
+
+	activeLine := newStandardLineForLineEngineTest("line-1", billing.LineEngineTypeInvoice, false)
+	deletedLine := newStandardLineForLineEngineTest("line-2", billing.LineEngineTypeInvoice, true)
+
+	sm := &InvoiceStateMachine{
+		Service: svc,
+		Invoice: billing.StandardInvoice{
+			StandardInvoiceBase: billing.StandardInvoiceBase{
+				Namespace: "ns",
+				ID:        "invoice-1",
+			},
+			Lines: billing.NewStandardInvoiceLines(billing.StandardLines{
+				activeLine,
+				deletedLine,
+			}),
+		},
+	}
+
+	require.NoError(t, sm.deleteInvoice(t.Context(), billing.DeleteInvoiceTriggerInput{
+		Source: billing.ChangeSourceSystem,
+	}))
+
+	require.NotNil(t, sm.Invoice.DeletedAt)
+	require.Equal(t, billing.ChangeSourceSystem, sm.Invoice.DeletionSource)
+	require.Len(t, invoiceEngine.deletedBySystemInputs, 1)
+	require.Equal(t, []string{"line-1"}, lineIDs(invoiceEngine.deletedBySystemInputs[0].Lines))
+}
+
+func TestDeleteInvoiceAPIRequestDoesNotDispatchSystemLineDeletion(t *testing.T) {
+	invoiceEngine := &recordingLineEngine{
+		NoopLineEngine: billingtestutils.NoopLineEngine{
+			EngineType: billing.LineEngineTypeInvoice,
+		},
+	}
+
+	svc := &Service{
+		lineEngines: newEngineRegistry(),
+	}
+	require.NoError(t, svc.RegisterLineEngine(invoiceEngine))
+
+	sm := &InvoiceStateMachine{
+		Service: svc,
+		Invoice: billing.StandardInvoice{
+			StandardInvoiceBase: billing.StandardInvoiceBase{
+				Namespace: "ns",
+				ID:        "invoice-1",
+			},
+			Lines: billing.NewStandardInvoiceLines(billing.StandardLines{
+				newStandardLineForLineEngineTest("line-1", billing.LineEngineTypeInvoice, true),
+			}),
+		},
+	}
+
+	require.NoError(t, sm.deleteInvoice(t.Context(), billing.DeleteInvoiceTriggerInput{
+		Source: billing.ChangeSourceAPIRequest,
+	}))
+
+	require.NotNil(t, sm.Invoice.DeletedAt)
+	require.Equal(t, billing.ChangeSourceAPIRequest, sm.Invoice.DeletionSource)
+	require.Empty(t, invoiceEngine.deletedBySystemInputs)
+}
+
 func TestEngineRegistryAllowsSingleCreateLineRouter(t *testing.T) {
 	registry := newEngineRegistry()
 	router := staticCreateLineRouter{engine: billing.LineEngineTypeChargeFlatFee}

openmeter/billing/service/stdinvoice.go

@@ -61,7 +61,7 @@ func (s *Service) UpdateStandardInvoice(ctx context.Context, input billing.Updat
 				// charges, so there is no extra line-engine notification for them here.
 				// Deletes still need the legacy deleted-by-system notification because
 				// the charge line updater currently relies on it to clean up realizations.
-				if err := s.dispatchSystemStandardLineDeletions(ctx, sm.Invoice, lineDiff); err != nil {
+				if err := s.dispatchSystemStandardLineDeletions(ctx, sm.Invoice, lineDiff.Deleted); err != nil {
 					return fmt.Errorf("dispatching system standard line deletions: %w", err)
 				}
 

openmeter/billing/service/stdinvoicestate.go

@@ -207,15 +207,15 @@ func allocateStateMachine() *InvoiceStateMachine {
 	stateMachine.Configure(billing.StandardInvoiceStatusDeleteInProgress).
 		Permit(billing.TriggerNext, billing.StandardInvoiceStatusDeleteSyncing).
 		Permit(billing.TriggerFailed, billing.StandardInvoiceStatusDeleteFailed).
-		OnActive(out.deleteInvoice)
+		OnEntry(statelessx.WithParameters(out.deleteInvoice))
 
 	stateMachine.Configure(billing.StandardInvoiceStatusDeleteSyncing).
 		Permit(billing.TriggerNext, billing.StandardInvoiceStatusDeleted).
 		Permit(billing.TriggerFailed, billing.StandardInvoiceStatusDeleteFailed).
 		OnActive(out.syncDeletedInvoice)
 
 	stateMachine.Configure(billing.StandardInvoiceStatusDeleteFailed).
-		Permit(billing.TriggerRetry, billing.StandardInvoiceStatusDeleteInProgress)
+		Permit(billing.TriggerDelete, billing.StandardInvoiceStatusDeleteInProgress)
 
 	stateMachine.Configure(billing.StandardInvoiceStatusDeleted)
 
@@ -397,8 +397,14 @@ func (m *InvoiceStateMachine) StatusDetails(ctx context.Context) (billing.Standa
 		outErr = errors.Join(outErr, err)
 	}
 
-	if availableActions.Delete, err = m.calculateAvailableActionDetails(ctx, billing.TriggerDelete); err != nil {
+	// Delete has trigger parameters and side effects, so status-details
+	// resolution must not fire it just to discover the resulting state.
+	if canDelete, err := m.StateMachine.CanFireCtx(ctx, billing.TriggerDelete); err != nil {
 		outErr = errors.Join(outErr, err)
+	} else if canDelete {
+		availableActions.Delete = &billing.StandardInvoiceAvailableActionDetails{
+			ResultingState: billing.StandardInvoiceStatusDeleted,
+		}
 	}
 
 	if availableActions.Retry, err = m.calculateAvailableActionDetails(ctx, billing.TriggerRetry); err != nil {
@@ -535,8 +541,8 @@ func (m *InvoiceStateMachine) AdvanceUntilStateStable(ctx context.Context) error
 	}
 }
 
-func (m *InvoiceStateMachine) CanFire(ctx context.Context, trigger billing.InvoiceTrigger) (bool, error) {
-	return m.StateMachine.CanFireCtx(ctx, trigger)
+func (m *InvoiceStateMachine) CanFire(ctx context.Context, trigger billing.InvoiceTrigger, args ...any) (bool, error) {
+	return m.StateMachine.CanFireCtx(ctx, trigger, args...)
 }
 
 func (m *InvoiceStateMachine) TriggerFailed(ctx context.Context) error {
@@ -555,9 +561,9 @@ func (m *InvoiceStateMachine) TriggerFailed(ctx context.Context) error {
 // FireAndActivate fires the trigger and activates the new state, if activation fails it automatically
 // transitions to the failed state and activates that.
 // In addition to the activation a calculation is always performed to ensure that the invoice is up to date.
-func (m *InvoiceStateMachine) FireAndActivate(ctx context.Context, trigger billing.InvoiceTrigger) error {
+func (m *InvoiceStateMachine) FireAndActivate(ctx context.Context, trigger billing.InvoiceTrigger, args ...any) error {
 	previousStatus := m.Invoice.Status
-	if err := m.StateMachine.FireCtx(ctx, trigger); err != nil {
+	if err := m.StateMachine.FireCtx(ctx, trigger, args...); err != nil {
 		if m.Invoice.Status == previousStatus {
 			return err
 		}
@@ -867,8 +873,48 @@ func (m *InvoiceStateMachine) syncDeletedInvoice(ctx context.Context) error {
 	})
 }
 
-// deleteInvoice deletes the invoice
-func (m *InvoiceStateMachine) deleteInvoice(ctx context.Context) error {
+// deleteInvoice handles entering the delete lifecycle. The delete source is
+// passed as trigger input so the state-machine side effect is tied to the
+// transition that requested deletion; the invoice field is only persisted for
+// audit.
+func (m *InvoiceStateMachine) deleteInvoice(ctx context.Context, input billing.DeleteInvoiceTriggerInput) error {
+	if err := input.Validate(); err != nil {
+		return err
+	}
+
+	// Each delete attempt owns its validation result. This keeps a retry from
+	// being poisoned by a prior delete-sync failure.
+	m.Invoice.ValidationIssues = nil
+
+	// DeletedAt is persisted before delete syncing starts. If syncing later fails
+	// and delete is retried, the source-specific line-engine cleanup has already
+	// run and must not be dispatched again.
+	if m.Invoice.DeletedAt != nil {
+		return nil
+	}
+
+	m.Invoice.DeletionSource = input.Source
+
+	switch input.Source {
+	case billing.ChangeSourceAPIRequest:
+		// API deletes are user initiated invoice deletes, not system line deletes.
+	case billing.ChangeSourceSystem:
+		// System invoice deletes keep the invoice lines visible on the deleted
+		// invoice, but charge-backed engines still need the same notification they
+		// receive when system code deletes standard lines individually.
+		if err := m.Service.dispatchSystemStandardLineDeletions(
+			ctx,
+			m.Invoice,
+			lo.Filter(m.Invoice.Lines.OrEmpty(), func(line *billing.StandardLine, _ int) bool {
+				return line != nil && line.DeletedAt == nil
+			}).AsGenericLines(),
+		); err != nil {
+			return err
+		}
+	default:
+		return fmt.Errorf("unsupported deletion source: %s", input.Source)
+	}
+
 	m.Invoice.DeletedAt = lo.ToPtr(clock.Now().In(time.UTC))
 
 	return nil