fix(returnmatch): dedicated log table, race-safe matching, spec/remark fixes (#338)

Review fixes on the #338 backend:

1. New table return_match_log (models/return_match_log.go, registered
   in migrations.MigratedModels()): match_id/action/operator/detail/
   created_at. sys_opera_log is a generic per-HTTP-call framework log,
   not queryable per match and not carrying operator/detail in a
   stable shape, so match/confirm/cancel/remark each write their own
   log row in the SAME transaction as the state change. Detail now
   returns the logs (newest first).

2. Race between matching and purchase creation: BatchMatch's outer
   screening pass (stage check outside any lock) is now followed by
   matchOneWithLock, which takes the same clause.Locking{Strength:
   "UPDATE"} lock on syb_product that purchase.Service.create takes,
   re-computes the stage inside that transaction via
   purchase.NewService(tx).ProcessStages, and returns
   errStageNoLongerEligible (surfaced as reasonCode
   stage_ineligible) if the product is no longer in a participating
   stage instead of inserting a stale match.
   TestMatchOneWithLock_SkipsWhenStageNoLongerParticipatesUnderLock
   covers the skip path.

3. matchOneWithLock now fills YeekeSpecText (raw variation_name) and
   PreviousProcessStage (the stage code at match time) on the
   inserted row. Remark takes an operator (for its log row) and
   rejects input over 500 runes with errRemarkTooLong instead of
   truncating (varchar(500) is a character-count limit in MySQL, so
   the check is utf8.RuneCountInString, not len()).

4. returnmatch.SYBSpecText now joins only non-empty color/size parts,
   so a single-dimension spec (e.g. color-only) no longer produces a
   stray leading/trailing comma ("黑色" instead of "黑色,"); matchKey
   additionally trims leading/trailing commas from both normalized
   sides via the new trimCommas() helper. New tests cover color-only
   and size-only matching through SelectMatches plus SYBSpecText/
   trimCommas directly.

5. Detail (service.go) now returns MatchDetail: SYBDetailView (order
   code, shopee item id, shop, title, target color/size, quantity,
   image, current computed stage+label), YeekeDetailView (return
   order sn, item id, variation id, shop, item name, variation name,
   quantity, image, destroy deadline), both sides' normalized spec
   text, the match row itself, and the operation logs — everything
   the prototype's compare screen (screen 3) needs.

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

server/app/goauto/migrations/migrate.go

@@ -80,6 +80,7 @@ func MigratedModels() []any {
 		&models.PDDProductReplacementItem{},
 		&models.PDDProductReplacementWorkerLease{},
 		&models.ReturnMatch{},
+		&models.ReturnMatchLog{},
 	}
 }
 

server/app/goauto/models/return_match_log.go

@@ -0,0 +1,28 @@
+package models
+
+import "time"
+
+// ReturnMatchLog is #338's dedicated operation log for the return-matching
+// feature. The framework's generic sys_opera_log records raw HTTP
+// request/response per call and is not queryable per match id nor does it
+// carry a stable action/operator/detail shape, so this table is written
+// explicitly, in the same transaction as the action it records (match /
+// confirm / cancel / remark).
+const (
+	ReturnMatchLogActionMatched   = "matched"
+	ReturnMatchLogActionConfirmed = "confirmed"
+	ReturnMatchLogActionCancelled = "cancelled"
+	ReturnMatchLogActionRemark    = "remark"
+)
+
+type ReturnMatchLog struct {
+	ID       uint64 `json:"id" gorm:"primaryKey;autoIncrement"`
+	MatchID  uint64 `json:"matchId" gorm:"not null;index"`
+	Action   string `json:"action" gorm:"size:16;not null;index;check:ck_return_match_log_action,action IN ('matched','confirmed','cancelled','remark')"`
+	Operator string `json:"operator" gorm:"size:64;not null;default:''"`
+	Detail   string `json:"detail" gorm:"size:1000;not null;default:''"`
+
+	CreatedAt time.Time `json:"createdAt"`
+}
+
+func (ReturnMatchLog) TableName() string { return "return_match_log" }

server/app/goauto/returnmatch/candidate.go

@@ -70,7 +70,7 @@ func SelectMatches(products []SYBCandidate, returns []ReturnCandidate, now time.
 		if !r.DestroyDeadline.After(now) {
 			continue
 		}
-		key := matchKey(r.ItemID, Normalize(r.VariationName))
+		key := matchKey(r.ItemID, trimCommas(Normalize(r.VariationName)))
 		buckets[key] = append(buckets[key], r)
 	}
 	for key := range buckets {
@@ -84,7 +84,7 @@ func SelectMatches(products []SYBCandidate, returns []ReturnCandidate, now time.
 	used := make(map[uint64]bool)
 	outcomes := make([]MatchOutcome, 0, len(products))
 	for _, p := range products {
-		key := matchKey(p.ShopeeItemID, Normalize(SYBSpecText(p.TargetColor, p.TargetSize)))
+		key := matchKey(p.ShopeeItemID, trimCommas(Normalize(SYBSpecText(p.TargetColor, p.TargetSize))))
 		var picked *ReturnCandidate
 		for i := range buckets[key] {
 			cand := buckets[key][i]

server/app/goauto/returnmatch/candidate_test.go

@@ -118,6 +118,26 @@ func TestSelectMatches_QuantityIgnored(t *testing.T) {
 	}
 }
 
+func TestSelectMatches_ColorOnlySYBSpecMatchesPlainYeekeText(t *testing.T) {
+	now := t1(0)
+	products := []SYBCandidate{{SYBProductID: 1, ShopeeItemID: "100", TargetColor: "黑色", TargetSize: "", CreatedAt: t1(-1)}}
+	returns := []ReturnCandidate{{ReturnItemID: 900, ItemID: "100", VariationName: "黑色", DestroyDeadline: t1(10)}}
+	out := SelectMatches(products, returns, now)
+	if !out[0].Matched || out[0].ReturnItemID != 900 {
+		t.Fatalf("color-only spec should still match: %+v", out[0])
+	}
+}
+
+func TestSelectMatches_SizeOnlySYBSpecMatchesPlainYeekeText(t *testing.T) {
+	now := t1(0)
+	products := []SYBCandidate{{SYBProductID: 1, ShopeeItemID: "100", TargetColor: "", TargetSize: "L", CreatedAt: t1(-1)}}
+	returns := []ReturnCandidate{{ReturnItemID: 900, ItemID: "100", VariationName: "L", DestroyDeadline: t1(10)}}
+	out := SelectMatches(products, returns, now)
+	if !out[0].Matched || out[0].ReturnItemID != 900 {
+		t.Fatalf("size-only spec should still match: %+v", out[0])
+	}
+}
+
 func TestSelectMatches_DifferentItemIDNeverMatches(t *testing.T) {
 	now := t1(0)
 	products := []SYBCandidate{{SYBProductID: 1, ShopeeItemID: "100", TargetColor: "白色", TargetSize: "L", CreatedAt: t1(-1)}}

server/app/goauto/returnmatch/handler.go

@@ -116,7 +116,7 @@ func (h Handler) Detail(c *gin.Context) {
 		c.JSON(http.StatusBadRequest, gin.H{"code": "INVALID_REQUEST", "message": "id 无效"})
 		return
 	}
-	match, err := NewService(db).Detail(c.Request.Context(), id)
+	detail, err := NewService(db).Detail(c.Request.Context(), id)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			c.JSON(http.StatusNotFound, gin.H{"code": "NOT_FOUND", "message": "匹配记录不存在"})
@@ -125,7 +125,7 @@ func (h Handler) Detail(c *gin.Context) {
 		c.JSON(http.StatusInternalServerError, gin.H{"code": "INTERNAL", "message": "服务端处理失败"})
 		return
 	}
-	c.JSON(http.StatusOK, gin.H{"code": 200, "data": gin.H{"item": match}})
+	c.JSON(http.StatusOK, gin.H{"code": 200, "data": detail})
 }
 
 func (h Handler) Confirm(c *gin.Context) {
@@ -192,12 +192,17 @@ func (h Handler) Remark(c *gin.Context) {
 		c.JSON(http.StatusBadRequest, gin.H{"code": "INVALID_REQUEST", "message": "请求体无效"})
 		return
 	}
-	match, err := NewService(db).Remark(c.Request.Context(), id, body.Remark)
+	_, operator := operatorFromContext(c)
+	match, err := NewService(db).Remark(c.Request.Context(), id, operator, body.Remark)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			c.JSON(http.StatusNotFound, gin.H{"code": "NOT_FOUND", "message": "匹配记录不存在"})
 			return
 		}
+		if errors.Is(err, errRemarkTooLong) {
+			c.JSON(http.StatusBadRequest, gin.H{"code": "REMARK_TOO_LONG", "message": "备注不能超过 500 字"})
+			return
+		}
 		c.JSON(http.StatusInternalServerError, gin.H{"code": "INTERNAL", "message": "服务端处理失败"})
 		return
 	}

server/app/goauto/returnmatch/normalize.go

@@ -94,7 +94,26 @@ func isSpace(r rune) bool {
 }
 
 // SYBSpecText builds the SYB-side comparable spec text from the parser's
-// target_color/target_size fields, per issue #338's data design.
+// target_color/target_size fields, per issue #338's data design. Only
+// non-empty parts are joined so a single-dimension spec (color-only or
+// size-only) doesn't pick up a stray leading/trailing comma that would
+// prevent it from normalizing equal to the yeeke side's single-value
+// variation_name (e.g. SYBSpecText("黑色", "") must be "黑色", not "黑色,").
 func SYBSpecText(targetColor, targetSize string) string {
-	return targetColor + "," + targetSize
+	parts := make([]string, 0, 2)
+	if targetColor != "" {
+		parts = append(parts, targetColor)
+	}
+	if targetSize != "" {
+		parts = append(parts, targetSize)
+	}
+	return strings.Join(parts, ",")
+}
+
+// trimCommas removes leading/trailing commas left over after normalization
+// (e.g. a source value that itself started or ended with a comma). Used only
+// when building the matching key, so exact-text comparisons/tests elsewhere
+// are unaffected.
+func trimCommas(s string) string {
+	return strings.Trim(s, ",")
 }

server/app/goauto/returnmatch/normalize_test.go

@@ -49,3 +49,30 @@ func TestSYBSpecText(t *testing.T) {
 		t.Fatalf("got %q", got)
 	}
 }
+
+func TestSYBSpecText_SingleDimension(t *testing.T) {
+	if got := SYBSpecText("黑色", ""); got != "黑色" {
+		t.Fatalf("color-only: got %q, want %q (no trailing comma)", got, "黑色")
+	}
+	if got := SYBSpecText("", "L"); got != "L" {
+		t.Fatalf("size-only: got %q, want %q (no leading comma)", got, "L")
+	}
+	if got := SYBSpecText("", ""); got != "" {
+		t.Fatalf("both empty: got %q, want empty", got)
+	}
+}
+
+func TestTrimCommas(t *testing.T) {
+	cases := map[string]string{
+		"黑色,":  "黑色",
+		",黑色":  "黑色",
+		",黑色,": "黑色",
+		"黑色":   "黑色",
+		"":     "",
+	}
+	for in, want := range cases {
+		if got := trimCommas(in); got != want {
+			t.Fatalf("trimCommas(%q) = %q, want %q", in, got, want)
+		}
+	}
+}

server/app/goauto/returnmatch/service.go

@@ -3,8 +3,10 @@ package returnmatch
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"go-admin/app/goauto/models"
 	"go-admin/app/goauto/purchase"
@@ -132,8 +134,19 @@ func (s *Service) BatchMatch(ctx context.Context, req BatchMatchRequest) (BatchM
 			resp.SkippedCount++
 			continue
 		}
-		match, insertErr := s.insertMatch(ctx, sybByID[id], outcome, req.Operator)
+		match, insertErr := s.matchOneWithLock(ctx, id, outcome, req.Operator)
 		if insertErr != nil {
+			if errors.Is(insertErr, errStageNoLongerEligible) {
+				// #338 review fix: the stage was re-checked under the same
+				// FOR UPDATE lock purchase.create takes, right before insert.
+				// A purchase task could have been created for this row
+				// between the outer screening pass above and this point;
+				// when that happens the product is no longer in a
+				// participating stage and this row is skipped, not matched.
+				resp.Items = append(resp.Items, BatchMatchItem{SYBProductID: id, Matched: false, ReasonCode: ReasonStageIneligible, Reason: "处理阶段已变化，不再参与匹配"})
+				resp.SkippedCount++
+				continue
+			}
 			if isUniqueConstraintErr(insertErr) {
 				// Rule 7 / acceptance item 9: a concurrent insert lost the
 				// race for either side — skip this row and report it, never
@@ -188,23 +201,66 @@ func (s *Service) availableReturnPool(ctx context.Context) ([]ReturnCandidate, e
 	return out, nil
 }
 
-func (s *Service) insertMatch(ctx context.Context, syb models.SYBProduct, outcome MatchOutcome, operator string) (models.ReturnMatch, error) {
-	sybID := syb.ID
-	returnID := outcome.ReturnItemID
-	now := s.Now()
-	match := models.ReturnMatch{
-		SYBProductID: sybID, YeekeReturnItemID: returnID,
-		ActiveSYBProductID: &sybID, ActiveYeekeReturnItemID: &returnID,
-		Status:        models.ReturnMatchStatusMatched,
-		SYBSpecText:   SYBSpecText(syb.TargetColor, syb.TargetSize),
-		NormalizedKey: outcome.NormalizedKey,
-		MatchedBy:     operator, MatchedAt: now,
-		DestroyDeadlineSnapshot: &outcome.DestroyDeadline,
-	}
-	if err := s.DB.WithContext(ctx).Create(&match).Error; err != nil {
-		return models.ReturnMatch{}, err
-	}
-	return match, nil
+// errStageNoLongerEligible is returned by matchOneWithLock when the SYB
+// product's process stage, re-checked under lock immediately before insert,
+// is no longer in participatingStages — e.g. a purchase task was created for
+// it between BatchMatch's outer screening pass and this point (#338 review
+// fix: race between matching and purchase creation).
+var errStageNoLongerEligible = errors.New("syb product stage no longer participates in matching")
+
+// matchOneWithLock takes the SAME row lock purchase.Service.create takes on
+// syb_product (clause.Locking{Strength: "UPDATE"}) and re-computes the
+// process stage inside that transaction via purchase.NewService(tx) before
+// inserting the match, so a purchase task creation racing with this batch
+// match can never both succeed: whichever gets the row lock first commits,
+// and the other sees the now-current state (an active return_match row, or
+// a task_created/order_review/purchase_succeeded stage) and is rejected/
+// skipped instead of double-committing an inconsistent state.
+func (s *Service) matchOneWithLock(ctx context.Context, sybID uint64, outcome MatchOutcome, operator string) (models.ReturnMatch, error) {
+	var match models.ReturnMatch
+	err := s.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+		var syb models.SYBProduct
+		if err := tx.Clauses(clauseLockUpdate()).First(&syb, sybID).Error; err != nil {
+			return err
+		}
+		stages, err := purchase.NewService(tx).ProcessStages(ctx, []uint64{sybID})
+		if err != nil {
+			return err
+		}
+		stage := stages[sybID]
+		if !participatingStages[stage.Stage] {
+			return errStageNoLongerEligible
+		}
+		var returnItem models.YeekeReturnItem
+		if err := tx.First(&returnItem, outcome.ReturnItemID).Error; err != nil {
+			return err
+		}
+		sybIDCopy := syb.ID
+		returnIDCopy := outcome.ReturnItemID
+		deadline := outcome.DestroyDeadline
+		now := s.Now()
+		match = models.ReturnMatch{
+			SYBProductID: sybIDCopy, YeekeReturnItemID: returnIDCopy,
+			ActiveSYBProductID: &sybIDCopy, ActiveYeekeReturnItemID: &returnIDCopy,
+			Status:                  models.ReturnMatchStatusMatched,
+			SYBSpecText:             SYBSpecText(syb.TargetColor, syb.TargetSize),
+			YeekeSpecText:           returnItem.VariationName,
+			NormalizedKey:           outcome.NormalizedKey,
+			PreviousProcessStage:    stage.Stage,
+			MatchedBy:               operator,
+			MatchedAt:               now,
+			DestroyDeadlineSnapshot: &deadline,
+		}
+		if err := tx.Create(&match).Error; err != nil {
+			return err
+		}
+		logRow := models.ReturnMatchLog{
+			MatchID: match.ID, Action: models.ReturnMatchLogActionMatched, Operator: operator,
+			Detail: fmt.Sprintf("匹配退货商品 #%d（%s），此前处理阶段：%s", returnIDCopy, returnItem.VariationName, stage.Label),
+		}
+		return tx.Create(&logRow).Error
+	})
+	return match, err
 }
 
 func isUniqueConstraintErr(err error) bool {
@@ -227,7 +283,10 @@ func (s *Service) Confirm(ctx context.Context, matchID uint64, operator string)
 		match.Status = models.ReturnMatchStatusConfirmed
 		match.ConfirmedBy = operator
 		match.ConfirmedAt = &now
-		return tx.Save(&match).Error
+		if err := tx.Save(&match).Error; err != nil {
+			return err
+		}
+		return tx.Create(&models.ReturnMatchLog{MatchID: match.ID, Action: models.ReturnMatchLogActionConfirmed, Operator: operator, Detail: "确认匹配，商品状态变为已用退货"}).Error
 	})
 	return match, err
 }
@@ -251,19 +310,35 @@ func (s *Service) Cancel(ctx context.Context, matchID uint64, operator string) (
 		match.ActiveYeekeReturnItemID = nil
 		match.CancelledBy = operator
 		match.CancelledAt = &now
-		return tx.Save(&match).Error
+		if err := tx.Save(&match).Error; err != nil {
+			return err
+		}
+		return tx.Create(&models.ReturnMatchLog{MatchID: match.ID, Action: models.ReturnMatchLogActionCancelled, Operator: operator, Detail: "取消匹配，SYB 商品恢复可采购，退货商品回到可用池"}).Error
 	})
 	return match, err
 }
 
-func (s *Service) Remark(ctx context.Context, matchID uint64, remark string) (models.ReturnMatch, error) {
+// errRemarkTooLong is returned instead of silently truncating; remark is a
+// varchar(500) column and MySQL VARCHAR length is a character count, so this
+// checks runes, not bytes.
+var errRemarkTooLong = errors.New("remark exceeds 500 characters")
+
+const maxRemarkLength = 500
+
+func (s *Service) Remark(ctx context.Context, matchID uint64, operator, remark string) (models.ReturnMatch, error) {
+	if utf8.RuneCountInString(remark) > maxRemarkLength {
+		return models.ReturnMatch{}, errRemarkTooLong
+	}
 	var match models.ReturnMatch
 	err := s.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
 		if err := tx.First(&match, matchID).Error; err != nil {
 			return err
 		}
 		match.Remark = remark
-		return tx.Save(&match).Error
+		if err := tx.Save(&match).Error; err != nil {
+			return err
+		}
+		return tx.Create(&models.ReturnMatchLog{MatchID: match.ID, Action: models.ReturnMatchLogActionRemark, Operator: operator, Detail: remark}).Error
 	})
 	return match, err
 }
@@ -292,10 +367,92 @@ func (s *Service) List(ctx context.Context, filter ListFilter) ([]models.ReturnM
 	return rows, err
 }
 
-func (s *Service) Detail(ctx context.Context, matchID uint64) (models.ReturnMatch, error) {
+// SYBDetailView/YeekeDetailView/MatchDetail back the prototype's compare
+// screen (screen 3): both sides' fields and images side by side, normalized
+// spec text for both, and the operation log.
+type SYBDetailView struct {
+	SYBProductID      uint64 `json:"sybProductId"`
+	OrderCode         string `json:"orderCode"`
+	ShopeeItemID      string `json:"shopeeItemId"`
+	ShopName          string `json:"shopName"`
+	ProductTitle      string `json:"productTitle"`
+	TargetColor       string `json:"targetColor"`
+	TargetSize        string `json:"targetSize"`
+	Quantity          int64  `json:"quantity"`
+	ImageURL          string `json:"imageUrl,omitempty"`
+	CurrentStage      string `json:"currentStage"`
+	CurrentStageLabel string `json:"currentStageLabel"`
+}
+
+type YeekeDetailView struct {
+	YeekeReturnItemID uint64     `json:"yeekeReturnItemId"`
+	OrderSN           string     `json:"orderSn"`
+	ItemID            string     `json:"itemId"`
+	VariationID       string     `json:"variationId"`
+	ShopName          string     `json:"shopName"`
+	ItemName          string     `json:"itemName"`
+	VariationName     string     `json:"variationName"`
+	Quantity          int64      `json:"quantity"`
+	Image             string     `json:"image,omitempty"`
+	DestroyDeadline   *time.Time `json:"destroyDeadline,omitempty"`
+}
+
+type MatchDetail struct {
+	Match               models.ReturnMatch      `json:"match"`
+	SYB                 *SYBDetailView          `json:"syb,omitempty"`
+	Yeeke               *YeekeDetailView        `json:"yeeke,omitempty"`
+	NormalizedSYBSpec   string                  `json:"normalizedSybSpec"`
+	NormalizedYeekeSpec string                  `json:"normalizedYeekeSpec"`
+	Logs                []models.ReturnMatchLog `json:"logs"`
+}
+
+func (s *Service) Detail(ctx context.Context, matchID uint64) (MatchDetail, error) {
 	var match models.ReturnMatch
-	err := s.DB.WithContext(ctx).First(&match, matchID).Error
-	return match, err
+	if err := s.DB.WithContext(ctx).First(&match, matchID).Error; err != nil {
+		return MatchDetail{}, err
+	}
+	detail := MatchDetail{Match: match}
+
+	var syb models.SYBProduct
+	if err := s.DB.WithContext(ctx).First(&syb, match.SYBProductID).Error; err == nil {
+		stages, stageErr := purchase.NewService(s.DB).ProcessStages(ctx, []uint64{syb.ID})
+		var stage purchase.ProcessStageResult
+		if stageErr == nil {
+			stage = stages[syb.ID]
+		}
+		detail.SYB = &SYBDetailView{
+			SYBProductID: syb.ID, OrderCode: syb.OrderCode, ShopeeItemID: syb.ShopeeItemID,
+			ShopName: syb.ShopName, ProductTitle: syb.ProductTitle,
+			TargetColor: syb.TargetColor, TargetSize: syb.TargetSize, Quantity: syb.Quantity,
+			ImageURL: syb.ImageURL, CurrentStage: stage.Stage, CurrentStageLabel: stage.Label,
+		}
+		detail.NormalizedSYBSpec = trimCommas(Normalize(SYBSpecText(syb.TargetColor, syb.TargetSize)))
+	} else if !errors.Is(err, gorm.ErrRecordNotFound) {
+		return detail, err
+	}
+
+	var item models.YeekeReturnItem
+	if err := s.DB.WithContext(ctx).First(&item, match.YeekeReturnItemID).Error; err == nil {
+		var pkg models.YeekeReturnPackage
+		if pkgErr := s.DB.WithContext(ctx).First(&pkg, item.PackageID).Error; pkgErr != nil && !errors.Is(pkgErr, gorm.ErrRecordNotFound) {
+			return detail, pkgErr
+		}
+		detail.Yeeke = &YeekeDetailView{
+			YeekeReturnItemID: item.ID, OrderSN: pkg.OrderSN, ItemID: item.ItemID, VariationID: item.VariationID,
+			ShopName: pkg.ShopName, ItemName: item.ItemName, VariationName: item.VariationName,
+			Quantity: item.Quantity, Image: item.Image, DestroyDeadline: pkg.DestroyDeadLine,
+		}
+		detail.NormalizedYeekeSpec = trimCommas(Normalize(item.VariationName))
+	} else if !errors.Is(err, gorm.ErrRecordNotFound) {
+		return detail, err
+	}
+
+	var logs []models.ReturnMatchLog
+	if err := s.DB.WithContext(ctx).Where("match_id = ?", matchID).Order("id DESC").Find(&logs).Error; err != nil {
+		return detail, err
+	}
+	detail.Logs = logs
+	return detail, nil
 }
 
 var errStateConflict = errors.New("return match state conflict")

server/app/goauto/returnmatch/service_test.go

@@ -2,6 +2,7 @@ package returnmatch
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -227,6 +228,173 @@ func TestConfirmThenCancel_RestoresAvailability(t *testing.T) {
 	}
 }
 
+// TestBatchMatch_SkipsWhenStageRacesToTaskCreatedUnderLock is the #338
+// review fix-2 regression: a purchase task gets created for the SYB product
+// (moving its stage to task_created, which does not participate) AFTER
+// BatchMatch's outer screening pass already judged it eligible but BEFORE
+// matchOneWithLock's own re-check runs. The row must be skipped with
+// stage_ineligible, not matched.
+func TestMatchOneWithLock_SkipsWhenStageNoLongerParticipatesUnderLock(t *testing.T) {
+	db := testDB(t)
+	s := NewService(db)
+	s.Now = func() time.Time { return time.Date(2026, 9, 24, 0, 0, 0, 0, time.UTC) }
+	deadline := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	syb := seedSYB(t, db, "SYB-1", 1, "白色", "L", time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC))
+	ret := seedReturn(t, db, "白色,L", &deadline)
+
+	// Simulate the exact race the review flagged: BatchMatch's outer
+	// screening pass already computed `outcome` while the product was still
+	// participating (pdd_unlinked). Before matchOneWithLock's own re-check
+	// runs (which takes the same row lock purchase.create takes and
+	// recomputes the stage inside that transaction), something else moves
+	// the product out of a participating stage — here simulated directly by
+	// flipping it to a parse-failed/manual_action state, same effect as a
+	// purchase task having been created for it in the meantime.
+	if err := db.Model(&models.SYBProduct{}).Where("id = ?", syb.ID).Update("parse_status", models.SYBParseStatusFailed).Error; err != nil {
+		t.Fatal(err)
+	}
+	outcome := MatchOutcome{SYBProductID: syb.ID, Matched: true, ReturnItemID: ret.ID, NormalizedKey: "100|白色,l"}
+
+	_, err := s.matchOneWithLock(context.Background(), syb.ID, outcome, "tester")
+	if !errors.Is(err, errStageNoLongerEligible) {
+		t.Fatalf("expected errStageNoLongerEligible, got %v", err)
+	}
+	var count int64
+	db.Model(&models.ReturnMatch{}).Where("syb_product_id = ?", syb.ID).Count(&count)
+	if count != 0 {
+		t.Fatalf("no return_match row should have been created when the stage race is caught: count=%d", count)
+	}
+}
+
+func TestBatchMatch_WritesLogRowInSameTransaction(t *testing.T) {
+	db := testDB(t)
+	s := NewService(db)
+	s.Now = func() time.Time { return time.Date(2026, 9, 24, 0, 0, 0, 0, time.UTC) }
+	deadline := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	syb := seedSYB(t, db, "SYB-1", 1, "白色", "L", time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC))
+	seedReturn(t, db, "白色,L【建議65-75公斤】", &deadline)
+
+	resp, err := s.BatchMatch(context.Background(), BatchMatchRequest{SYBProductIDs: []uint64{syb.ID}, Operator: "tester"})
+	if err != nil || resp.MatchedCount != 1 {
+		t.Fatalf("setup match failed: %v %+v", err, resp)
+	}
+	var logs []models.ReturnMatchLog
+	if err := db.Where("match_id = ?", resp.Items[0].MatchID).Find(&logs).Error; err != nil {
+		t.Fatal(err)
+	}
+	if len(logs) != 1 || logs[0].Action != models.ReturnMatchLogActionMatched || logs[0].Operator != "tester" {
+		t.Fatalf("expected one matched log row: %+v", logs)
+	}
+}
+
+func TestConfirmCancelRemark_EachWritesOwnLogRow(t *testing.T) {
+	db := testDB(t)
+	s := NewService(db)
+	s.Now = func() time.Time { return time.Date(2026, 9, 24, 0, 0, 0, 0, time.UTC) }
+	deadline := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	syb := seedSYB(t, db, "SYB-1", 1, "白色", "L", time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC))
+	seedReturn(t, db, "白色,L", &deadline)
+	resp, _ := s.BatchMatch(context.Background(), BatchMatchRequest{SYBProductIDs: []uint64{syb.ID}, Operator: "matcher"})
+	matchID := resp.Items[0].MatchID
+
+	if _, err := s.Remark(context.Background(), matchID, "reviewer", "看起来没问题"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Confirm(context.Background(), matchID, "reviewer"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Cancel(context.Background(), matchID, "reviewer2"); err != nil {
+		t.Fatal(err)
+	}
+
+	var logs []models.ReturnMatchLog
+	if err := db.Where("match_id = ?", matchID).Order("id ASC").Find(&logs).Error; err != nil {
+		t.Fatal(err)
+	}
+	wantActions := []string{models.ReturnMatchLogActionMatched, models.ReturnMatchLogActionRemark, models.ReturnMatchLogActionConfirmed, models.ReturnMatchLogActionCancelled}
+	if len(logs) != len(wantActions) {
+		t.Fatalf("expected %d log rows, got %d: %+v", len(wantActions), len(logs), logs)
+	}
+	for i, action := range wantActions {
+		if logs[i].Action != action {
+			t.Fatalf("log[%d].Action = %q, want %q (%+v)", i, logs[i].Action, action, logs)
+		}
+	}
+	if logs[1].Detail != "看起来没问题" {
+		t.Fatalf("remark log must carry the remark text: %+v", logs[1])
+	}
+}
+
+func TestRemark_RejectsOverLongInputInsteadOfTruncating(t *testing.T) {
+	db := testDB(t)
+	s := NewService(db)
+	s.Now = func() time.Time { return time.Date(2026, 9, 24, 0, 0, 0, 0, time.UTC) }
+	deadline := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	syb := seedSYB(t, db, "SYB-1", 1, "白色", "L", time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC))
+	seedReturn(t, db, "白色,L", &deadline)
+	resp, _ := s.BatchMatch(context.Background(), BatchMatchRequest{SYBProductIDs: []uint64{syb.ID}})
+	matchID := resp.Items[0].MatchID
+
+	tooLong := strings.Repeat("字", 501)
+	if _, err := s.Remark(context.Background(), matchID, "op", tooLong); !errors.Is(err, errRemarkTooLong) {
+		t.Fatalf("expected errRemarkTooLong, got %v", err)
+	}
+	var match models.ReturnMatch
+	db.First(&match, matchID)
+	if match.Remark != "" {
+		t.Fatalf("rejected remark must not be saved (partially or fully): %q", match.Remark)
+	}
+
+	exactly500 := strings.Repeat("字", 500)
+	if _, err := s.Remark(context.Background(), matchID, "op", exactly500); err != nil {
+		t.Fatalf("exactly 500 runes must be accepted: %v", err)
+	}
+}
+
+func TestDetail_ReturnsBothSidesNormalizedSpecsAndLogs(t *testing.T) {
+	db := testDB(t)
+	s := NewService(db)
+	s.Now = func() time.Time { return time.Date(2026, 9, 24, 0, 0, 0, 0, time.UTC) }
+	deadline := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)
+	syb := seedSYB(t, db, "SYB-1", 1, "白色", "L", time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC))
+	ret := seedReturn(t, db, "白色,L【建議65-75公斤】", &deadline)
+
+	resp, err := s.BatchMatch(context.Background(), BatchMatchRequest{SYBProductIDs: []uint64{syb.ID}, Operator: "matcher"})
+	if err != nil || resp.MatchedCount != 1 {
+		t.Fatalf("setup failed: %v %+v", err, resp)
+	}
+	matchID := resp.Items[0].MatchID
+	if _, err := s.Remark(context.Background(), matchID, "op", "备注文字"); err != nil {
+		t.Fatal(err)
+	}
+
+	detail, err := s.Detail(context.Background(), matchID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if detail.SYB == nil || detail.SYB.SYBProductID != syb.ID || detail.SYB.OrderCode != "SYB-1" {
+		t.Fatalf("missing/wrong SYB detail: %+v", detail.SYB)
+	}
+	if detail.Yeeke == nil || detail.Yeeke.YeekeReturnItemID != ret.ID || detail.Yeeke.OrderSN != "ORD1" {
+		t.Fatalf("missing/wrong yeeke detail: %+v", detail.Yeeke)
+	}
+	if detail.Yeeke.DestroyDeadline == nil || !detail.Yeeke.DestroyDeadline.Equal(deadline) {
+		t.Fatalf("expected destroy deadline to be carried through: %+v", detail.Yeeke)
+	}
+	if detail.NormalizedSYBSpec == "" || detail.NormalizedSYBSpec != detail.NormalizedYeekeSpec {
+		t.Fatalf("expected both sides to normalize equal: syb=%q yeeke=%q", detail.NormalizedSYBSpec, detail.NormalizedYeekeSpec)
+	}
+	if len(detail.Logs) != 2 {
+		t.Fatalf("expected 2 log rows (matched + remark), got %+v", detail.Logs)
+	}
+	if detail.Logs[0].Action != models.ReturnMatchLogActionRemark {
+		t.Fatalf("expected newest-first ordering (remark first): %+v", detail.Logs)
+	}
+	if detail.Match.YeekeSpecText == "" || detail.Match.PreviousProcessStage == "" {
+		t.Fatalf("insertMatch must fill YeekeSpecText and PreviousProcessStage: %+v", detail.Match)
+	}
+}
+
 // TestBatchMatch_ConcurrentInsertSkipsConflictRow exercises the DB unique
 // constraint directly: two goroutines racing to insert an active match for
 // the exact same SYB product must have exactly one winner, and the loser