fix: close price-tamper, premature rider-free, and IST/txn gaps in merged cx/handover work

Reviewed the 10 merged customer-app/base-handover commits and fixed the
defects found:

- HIGH (money): CreateCxBooking let the request body's `estimate` set the
  billed price with no server-side check; it flows into Estimatedprice →
  ridercharges (miler pay + tenant bill) with no weight re-price, so
  {min:1,max:1} settled a delivery at ₹1. Now the client estimate is honoured
  only when it matches the server quote within 15%, else the server quote
  stands.
- MED: base handover freed the rider and closed the booking-level assignment
  after the FIRST parcel of a multi-destination pickup, dropping the remaining
  stops and crediting one leg. Now finalized only when no consignment of the
  booking is still in the rider's hands.
- MED: inwardedat/completedat were written with time.Now() (UTC) instead of
  DBNow() (IST), skewing them ~5h30 vs createdat and the earnings/reconcile
  windows. Fixed in the handover, inbound-scan, reconcile and pickup-complete
  paths.
- MED: B2C customers got two "miler assigned" pushes on auto-assign (two token
  stores) and none on manual assign. Reconciled to one cxstage.Notify on both
  paths.
- LOW: ReconcileHubInbound now runs in a transaction and checks its audit
  inserts (was returning 200 with a silently-missing history row); CxLogout no
  longer reports signedOut when the token revoke fails; a rider-named handover
  base far from their reported position is rejected instead of silently
  rerouting the parcel to another city.

go build, go vet and go test ./... all pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WRaFH5hMRqmUQvVPQsyjZD

controllers/booking_assignment_service.go

@@ -121,6 +121,12 @@ func publishAssignmentUpdate(booking *models.PickupBooking, milerUserID int) {
 				"miler_id", milerUserID, "booking_id", booking.Bookingid, "error", err)
 		}
 	}
+
+	// Customer push, the same path auto-assign uses (cxstage → doormile_cx device
+	// tokens). Manual assignment recorded the CxStageAssigned stage but sent the
+	// customer nothing, so a console/hub assignment left the customer with no
+	// "miler assigned" notification while auto-assign sent one.
+	cxstage.Notify(booking.Bookingid, nil, constants.CxStageAssigned)
 }
 
 // AssignMilerToBooking is the single source of truth for manually assigning a

controllers/cxAuthController.go

@@ -504,9 +504,14 @@ func CxLogout(c *fiber.Ctx) error {
 
 	now := time.Now()
 	if strings.TrimSpace(req.RefreshToken) != "" {
-		db.DB.Model(&models.CustomerRefreshToken{}).
+		// A failed revoke must not answer signedOut — the 60-day refresh token
+		// would stay valid while the customer believes they logged out.
+		if err := db.DB.Model(&models.CustomerRefreshToken{}).
 			Where("tokenhash = ? AND appcustomerid = ?", hashToken(req.RefreshToken), customerID).
-			Update("revokedat", now)
+			Update("revokedat", now).Error; err != nil {
+			utils.Error("CxLogout: could not revoke the presented token", "customer_id", customerID, "error", err)
+			return utils.CxInternal(c)
+		}
 	} else {
 		// No token supplied — sign out everywhere rather than leave a session
 		// the customer believes they ended.
@@ -514,8 +519,12 @@ func CxLogout(c *fiber.Ctx) error {
 	}
 
 	if strings.TrimSpace(req.DeviceToken) != "" {
-		db.DB.Where("appcustomerid = ? AND token = ?", customerID, req.DeviceToken).
-			Delete(&models.CustomerDevice{})
+		if err := db.DB.Where("appcustomerid = ? AND token = ?", customerID, req.DeviceToken).
+			Delete(&models.CustomerDevice{}).Error; err != nil {
+			// The session is already revoked above; a stuck device row only means a
+			// stray push, so log and still report signed out rather than fail.
+			utils.Warn("CxLogout: could not remove device token", "customer_id", customerID, "error", err)
+		}
 	}
 
 	return utils.CxOK(c, fiber.Map{"signedOut": true})

controllers/cxBookingController.go

@@ -91,6 +91,32 @@ type cxCreateBookingRequest struct {
 	Remarks string `json:"remarks"`
 }
 
+// cxEstimateMatchesQuote reports whether a customer-supplied price band is close
+// enough to the server's own quote to be trusted as the agreed price. It guards
+// the stored Estimatedprice — which becomes ridercharges at completion — against
+// a tampered request body while still honouring an honest estimate that came
+// from our estimate endpoint. Rejects negatives, inverted bands, and — when the
+// server could not price the pickup (zero quote) — any client number at all,
+// since with no server figure to check against the client's would be unbounded.
+func cxEstimateMatchesQuote(clientMin, clientMax, quoteMin, quoteMax int) bool {
+	if clientMin < 0 || clientMax < clientMin {
+		return false
+	}
+	serverMid := float64(quoteMin+quoteMax) / 2
+	if serverMid <= 0 {
+		return false
+	}
+	clientMid := float64(clientMin+clientMax) / 2
+	diff := clientMid - serverMid
+	if diff < 0 {
+		diff = -diff
+	}
+	// 15% of the server midpoint absorbs rounding and minor pricing drift between
+	// the estimate call and confirm, without letting a materially different
+	// number through.
+	return diff <= 0.15*serverMid
+}
+
 // CreateCxBooking creates the pickup.
 func CreateCxBooking(c *fiber.Ctx) error {
 	customerID := c.Locals("userid").(int)
@@ -228,9 +254,22 @@ func CreateCxBooking(c *fiber.Ctx) error {
 	quote := quoteCxPickup(req.Pickup.Lat, req.Pickup.Lng, estimateDestinations)
 	estimateMin, estimateMax := quote.Min, quote.Max
 	if req.Estimate != nil && req.Estimate.Max > 0 {
-		// The customer's number wins. They agreed to what was on their screen,
-		// and re-pricing at confirm time would quietly change the deal.
-		estimateMin, estimateMax = req.Estimate.Min, req.Estimate.Max
+		if cxEstimateMatchesQuote(req.Estimate.Min, req.Estimate.Max, quote.Min, quote.Max) {
+			// The customer's number wins — but only when it agrees with what the
+			// server independently prices for this pickup. An honest app took its
+			// estimate from our own estimate endpoint, so it matches; re-pricing at
+			// confirm time would quietly change the deal for that customer. A
+			// tampered body (e.g. {min:1,max:1}) does NOT match and must never
+			// stand, because this midpoint becomes Estimatedprice, which the pickup
+			// and every handover leg copy verbatim into bookingassignments.ridercharges
+			// — the miler's pay and the tenant's bill — with no weight re-price.
+			estimateMin, estimateMax = req.Estimate.Min, req.Estimate.Max
+		} else {
+			utils.Warn("CreateCxBooking: client estimate rejected, pricing from server quote",
+				"customer_id", customerID,
+				"client_min", req.Estimate.Min, "client_max", req.Estimate.Max,
+				"quote_min", quote.Min, "quote_max", quote.Max)
+		}
 	}
 
 	now := utils.DBNow()

controllers/hubController.go

@@ -561,7 +561,9 @@ func CreateInboundScan(c *fiber.Ctx) error {
 		}
 	}
 
-	now := time.Now()
+	// IST wall-clock (DBNow), so inwardedat matches createdat/updatedat and the
+	// earnings/reconciliation windows that compare against it.
+	now := utils.DBNow()
 	consignment.Status = constants.ConsignmentInwardedAtHub
 	consignment.Currenthubid = &hubID
 	consignment.Condition = req.Condition

controllers/hubInboundController.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-	"time"
 
 	"doormile/constants"
 	"doormile/db"
@@ -204,7 +203,11 @@ func ReconcileHubInbound(c *fiber.Ctx) error {
 		return utils.NotFound(c, "consignment not found")
 	}
 
-	now := time.Now()
+	// IST wall-clock (DBNow) to match the other consignment timestamps. A single
+	// transaction so a failed audit insert can't leave a state change with no
+	// history row and still answer 200.
+	now := utils.DBNow()
+	tx := db.DB.Begin()
 
 	if !*req.Received {
 		description := strings.TrimSpace(req.Remarks)
@@ -224,16 +227,23 @@ func ReconcileHubInbound(c *fiber.Ctx) error {
 			Status:        constants.ExceptionOpen,
 			Createdby:     staffAccountID,
 		}
-		if err := db.DB.Create(&exception).Error; err != nil {
+		if err := tx.Create(&exception).Error; err != nil {
+			tx.Rollback()
 			return utils.Internal(c, "failed to raise handover exception")
 		}
-		db.DB.Create(&models.ConsignmentHistory{
+		if err := tx.Create(&models.ConsignmentHistory{
 			Consignmentid: consignment.Consignmentid,
 			Hubid:         &hubID,
 			Eventstatus:   consignment.Status,
 			Remarks: fmt.Sprintf("Handover disputed at base by hub staff account %d: %s",
 				staffAccountID, description),
-		})
+		}).Error; err != nil {
+			tx.Rollback()
+			return utils.Internal(c, "failed to record dispute history")
+		}
+		if err := tx.Commit().Error; err != nil {
+			return utils.Internal(c, "failed to record the dispute")
+		}
 
 		return utils.OK(c, fiber.Map{
 			"consignmentid":     consignment.Consignmentid,
@@ -258,7 +268,8 @@ func ReconcileHubInbound(c *fiber.Ctx) error {
 	if consignment.Inwardedat == nil {
 		consignment.Inwardedat = &now
 	}
-	if err := db.DB.Save(&consignment).Error; err != nil {
+	if err := tx.Save(&consignment).Error; err != nil {
+		tx.Rollback()
 		return utils.Internal(c, "failed to record receipt")
 	}
 
@@ -267,12 +278,19 @@ func ReconcileHubInbound(c *fiber.Ctx) error {
 		if remarks == "" {
 			remarks = "Physical receipt confirmed at base"
 		}
-		db.DB.Create(&models.ConsignmentHistory{
+		if err := tx.Create(&models.ConsignmentHistory{
 			Consignmentid: consignment.Consignmentid,
 			Hubid:         &hubID,
 			Eventstatus:   constants.ConsignmentInwardedAtHub,
 			Remarks:       fmt.Sprintf("%s (hub staff account %d)", remarks, staffAccountID),
-		})
+		}).Error; err != nil {
+			tx.Rollback()
+			return utils.Internal(c, "failed to record receipt history")
+		}
+	}
+
+	if err := tx.Commit().Error; err != nil {
+		return utils.Internal(c, "failed to record the receipt")
 	}
 
 	return utils.OK(c, fiber.Map{

controllers/logisticsHandoverController.go

@@ -7,7 +7,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"time"
 
 	"doormile/constants"
 	"doormile/db"
@@ -26,6 +25,12 @@ import (
 // the app's wording, and nothing in the app's wording should leak back in here.
 // --------------------
 
+// maxHandoverBaseKM bounds how far a rider may be from a base they EXPLICITLY
+// name at handover. A rider stands at the base they hand into, so a named base
+// this far from their reported position is a wrong id (a different city), not a
+// real handover. Generous enough to never reject two bases in one metro.
+const maxHandoverBaseKM = 50.0
+
 // hubHandoverEnabled gates the two-step hub flow: a hub-routed parcel stops at
 // Created — collected, in the rider's hands, on its way to a base — and only
 // reaches Inwarded_at_Hub when the handover is actually recorded, by the rider
@@ -372,6 +377,32 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error {
 		return utils.Fail(c, fiber.StatusNotFound, constants.ErrHubNotFound, "hub_id does not match a known base")
 	}
 
+	lat, lon := 0.0, 0.0
+	if req.Latitude != nil {
+		lat = *req.Latitude
+	} else if req.Lat != nil {
+		lat = *req.Lat
+	}
+	if req.Longitude != nil {
+		lon = *req.Longitude
+	} else if req.Lon != nil {
+		lon = *req.Lon
+	}
+
+	// Guard a fat-fingered base id from silently rerouting the parcel to a base in
+	// the wrong city. Only a base the rider EXPLICITLY names (not the routed
+	// default) is checked, and only when they report their position and the base
+	// has real coordinates: a rider is physically at the base they hand into, so a
+	// named base far from where they stand is a wrong id, not a real handover.
+	riderNamedHub := req.HubID != nil || req.HubIDAlt != nil
+	routedHub := consignment.Currenthubid != nil && hub.Hubid == *consignment.Currenthubid
+	if riderNamedHub && !routedHub && (lat != 0 || lon != 0) && hub.Latitude != 0 && hub.Longitude != 0 {
+		if km := haversineKM(lat, lon, hub.Latitude, hub.Longitude); km > maxHandoverBaseKM {
+			return utils.Fail(c, fiber.StatusBadRequest, constants.ErrInvalidState,
+				fmt.Sprintf("selected base %s is %.0f km from your location — check the base before handing over", hub.Hubname, km))
+		}
+	}
+
 	// Already inwarded: answer with the state that stands rather than failing, so
 	// a retry after a dropped response confirms rather than errors. This is also
 	// what a rider on the compatibility flow hits every time — there,
@@ -397,19 +428,10 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error {
 			fmt.Sprintf("consignment is %s — it cannot be handed over at a base from this state", consignment.Status))
 	}
 
-	lat, lon := 0.0, 0.0
-	if req.Latitude != nil {
-		lat = *req.Latitude
-	} else if req.Lat != nil {
-		lat = *req.Lat
-	}
-	if req.Longitude != nil {
-		lon = *req.Longitude
-	} else if req.Lon != nil {
-		lon = *req.Lon
-	}
-
-	now := time.Now()
+	// IST wall-clock, matching createdat/updatedat and the DBNow() convention, so
+	// inwardedat lines up with the other timestamps base reconciliation and the
+	// earnings "today" window compare it against.
+	now := utils.DBNow()
 	tx := db.DB.Begin()
 
 	consignment.Status = constants.ConsignmentInwardedAtHub
@@ -448,40 +470,69 @@ func MilerInwardConsignmentAtHub(c *fiber.Ctx) error {
 	// multi-destination pickup, so joining on it found nothing for orders 2..N
 	// — and an intercity rider handing in the second parcel of a three-stop
 	// pickup had their assignment left open and their distance recorded as zero.
+	// Close the rider's booking-level assignment and free them ONLY once every
+	// parcel from this pickup has left their hands. A customer-app booking is one
+	// booking → N destinations → N consignments but a single BookingAssignment;
+	// closing on the FIRST handover freed the rider and dropped the remaining
+	// stops from the sequencer while parcels 2..N were still on them, crediting
+	// only the first leg. So finalize only when no consignment of this booking is
+	// still in a rider-carrying state (this one is already Inwarded_at_Hub above).
+	finalizeRiderLeg := true
 	if _, bookingPtr, ok := cxDestinationForConsignment(consignment.Consignmentid); ok && bookingPtr != nil {
 		booking := *bookingPtr
-		dropLat, dropLon := lat, lon
-		if dropLat == 0 && dropLon == 0 {
-			dropLat, dropLon = hub.Latitude, hub.Longitude
-		}
-		riderKms := haversineKM(consignment.Pickuplatitude, consignment.Pickuplongitude, dropLat, dropLon)
 
-		orderAmount := 0.0
-		var serviceOpt models.BookingServiceOption
-		if tx.Where("bookingid = ?", booking.Bookingid).Order("createdat DESC").
-			First(&serviceOpt).Error == nil {
-			orderAmount = serviceOpt.Estimatedprice
-		}
-
-		if err := tx.Model(&models.BookingAssignment{}).
-			Where("bookingid = ? AND mileruserid = ? AND assignmentstatus IN ?",
-				booking.Bookingid, milerUserID,
-				[]string{constants.AssignmentAssigned, constants.AssignmentAccepted}).
-			Updates(map[string]interface{}{
-				"assignmentstatus": constants.AssignmentCompleted,
-				"completedat":      now,
-				"riderkms":         riderKms,
-				"ridercharges":     orderAmount,
-			}).Error; err != nil {
+		var carrying int64
+		if err := tx.Model(&models.Consignment{}).
+			Joins("JOIN bookingdestinations bd ON bd.consignmentid = consignments.consignmentid").
+			Where("bd.bookingid = ? AND consignments.status IN ?",
+				booking.Bookingid,
+				[]string{constants.ConsignmentCreated, constants.ConsignmentCollectedByMiler, constants.ConsignmentOutForDelivery}).
+			Count(&carrying).Error; err != nil {
 			tx.Rollback()
-			return utils.Internal(c, "failed to close assignment")
+			return utils.Internal(c, "failed to check the booking's remaining parcels")
+		}
+
+		if carrying > 0 {
+			// Rider still carries other parcels from this pickup: leave the
+			// assignment open and the rider on the job. The leg is credited and the
+			// rider freed at the final handover.
+			finalizeRiderLeg = false
+		} else {
+			dropLat, dropLon := lat, lon
+			if dropLat == 0 && dropLon == 0 {
+				dropLat, dropLon = hub.Latitude, hub.Longitude
+			}
+			riderKms := haversineKM(consignment.Pickuplatitude, consignment.Pickuplongitude, dropLat, dropLon)
+
+			orderAmount := 0.0
+			var serviceOpt models.BookingServiceOption
+			if tx.Where("bookingid = ?", booking.Bookingid).Order("createdat DESC").
+				First(&serviceOpt).Error == nil {
+				orderAmount = serviceOpt.Estimatedprice
+			}
+
+			if err := tx.Model(&models.BookingAssignment{}).
+				Where("bookingid = ? AND mileruserid = ? AND assignmentstatus IN ?",
+					booking.Bookingid, milerUserID,
+					[]string{constants.AssignmentAssigned, constants.AssignmentAccepted}).
+				Updates(map[string]interface{}{
+					"assignmentstatus": constants.AssignmentCompleted,
+					"completedat":      now,
+					"riderkms":         riderKms,
+					"ridercharges":     orderAmount,
+				}).Error; err != nil {
+				tx.Rollback()
+				return utils.Internal(c, "failed to close assignment")
+			}
 		}
 	}
 
-	if err := tx.Model(&models.MilerProfile{}).Where("userid = ?", milerUserID).
-		Update("availabilitystatus", constants.MilerAvailable).Error; err != nil {
-		tx.Rollback()
-		return utils.Internal(c, "failed to update miler availability")
+	if finalizeRiderLeg {
+		if err := tx.Model(&models.MilerProfile{}).Where("userid = ?", milerUserID).
+			Update("availabilitystatus", constants.MilerAvailable).Error; err != nil {
+			tx.Rollback()
+			return utils.Internal(c, "failed to update miler availability")
+		}
 	}
 
 	// The customer's "In transit" milestone. Recorded against THIS order, not

internal/assignment/crm_assignment.go

@@ -256,7 +256,9 @@ func commitAssignment(booking *models.PickupBooking, candidate *milerCandidate,
 
 	publishAssignment(booking, milerUserID)
 	notifyMilerNewAssignment(candidate.profile, booking.Bookingid)
-	notifyCustomerMilerAssigned(booking, candidate.profile.Displayname)
+	// Customer push goes through cxstage.Notify above (the doormile_cx device
+	// tokens). notifyCustomerMilerAssigned targeted the older AppCustomer.Devicetoken
+	// and firing both sent the customer two near-identical "miler assigned" pushes.
 
 	// Order the rider's stops now this one is added. No-op below two active stops;
 	// best-effort and off this goroutine's critical path.