Sales: AddOrderItemHandler saves inside its transaction delegate with no rollback cleanup for tracked entities #743

Found by the invariants review seat on PR #742 (#722), 2026-09-10. Not reachable today — filed so the next editor of that delegate finds it.

What

AddOrderItemHandler.HandleAsync now calls unitOfWork.SaveChangesAsync(token) inside the ExecuteInTransactionAsync delegate, so the audit row can carry the SalesOrderItem.Id EF assigns at that save (#722).

Of the ExecuteInTransactionAsync call sites, only this one and UpdateFarmSettingsHandler save inside the delegate. UpdateFarmSettingsHandler pairs that with a cleanup on the !committed branch:

if (!committed)
{
    // The rollback undid the row, but the tracked entity still carries
    // the new currency; anything that saves later in this request — the
    // idempotency record, for one — would flush it (pi review of #159).
    accounts.DiscardChanges(account);

AddOrderItemHandler has no equivalent.

Why it is not a bug today

The delegate's only return false sits above the inner save, and all three of SalesOrder.AddItem's failure paths return before _items.Add (SalesOrder.cs:48-58 vs :62). So on the only non-committing path, nothing has been mutated and there is nothing to discard. Verified by reading, and the transaction guarantee itself is pinned by AddItem_WhenTheAuditWriteFails_RollsBackTheLine plus mutation row M4.

Why it is worth filing anyway

The shape is worse than #159's if it ever becomes reachable. After the inner save the order and the new item are Unchanged, not Modified — so a rollback leaves them looking persisted, and a later flush in the same request would not re-write them, it would silently drop them. #159's DiscardChanges exists to make that loud.

Adding any return false, or any catchable throw, below the inner save reopens it, and nothing in the suite fails when someone does.

Options

1. A guard asserting no return false follows the inner save in that delegate.
2. A DiscardChanges-style cleanup on the !committed branch, mirroring UpdateFarmSettingsHandler.
3. Won't-fix, recorded — the constraint lives only in the code comment that is already there.

Not fixed in #722 because it is unreachable there and the fix is new logic, which would earn its own review round for a defect nobody can currently trigger.