fix(id): always compare legacy IDs below newly formatted IDs

platform/base/id/id.go

@@ -18,6 +18,7 @@ package id
 import (
 	"fmt"
 	"strconv"
+	"strings"
 )
 
 // FromCounter returns the canonical decimal resource ID for value.
@@ -44,17 +45,39 @@ func Validate(id string) error {
 	return nil
 }
 
-// Compare compares two canonical resource IDs numerically.
+// Compare orders legacy IDs by their numeric suffix, followed by decimal IDs numerically.
+// Callers must ensure both IDs belong to the same queue and resource kind.
+// This ordering requires a one-way writer switch from legacy to decimal IDs.
 func Compare(a, b string) (int, error) {
-	aValue, err := parseResourceID(a)
+	aCounter, bCounter := a, b
+	aSeparator := strings.LastIndexByte(a, '/')
+	bSeparator := strings.LastIndexByte(b, '/')
+	aLegacy, bLegacy := aSeparator >= 0, bSeparator >= 0
+	if aLegacy {
+		if aSeparator == 0 {
+			return 0, fmt.Errorf("invalid first resource ID %q: legacy prefix must not be empty", a)
+		}
+		aCounter = a[aSeparator+1:]
+	}
+	if bLegacy {
+		if bSeparator == 0 {
+			return 0, fmt.Errorf("invalid second resource ID %q: legacy prefix must not be empty", b)
+		}
+		bCounter = b[bSeparator+1:]
+	}
+	aValue, err := parseResourceID(aCounter)
 	if err != nil {
 		return 0, fmt.Errorf("invalid first resource ID %q: %w", a, err)
 	}
-	bValue, err := parseResourceID(b)
+	bValue, err := parseResourceID(bCounter)
 	if err != nil {
 		return 0, fmt.Errorf("invalid second resource ID %q: %w", b, err)
 	}
 	switch {
+	case aLegacy && !bLegacy:
+		return -1, nil
+	case !aLegacy && bLegacy:
+		return 1, nil
 	case aValue < bValue:
 		return -1, nil
 	case aValue > bValue:

platform/base/id/id_test.go

@@ -88,8 +88,30 @@ func TestCompare(t *testing.T) {
 		{name: "older", a: "9", b: "10", want: -1},
 		{name: "equal", a: "42", b: "42"},
 		{name: "newer", a: "10", b: "9", want: 1},
-		{name: "invalid first", a: "queue/9", b: "10", wantErr: true},
+		{name: "invalid first", a: "bad", b: "10", wantErr: true},
 		{name: "invalid second", a: "9", b: "batch.10", wantErr: true},
+		{name: "legacy counters sort numerically", a: "queue/9", b: "queue/10", want: -1},
+		{name: "newer legacy counter", a: "queue/10", b: "queue/9", want: 1},
+		{name: "decimal is newer despite smaller counter", a: "1", b: "queue/42", want: 1},
+		{name: "legacy is older despite larger counter", a: "queue/42", b: "1", want: -1},
+		{name: "same counter across formats is not equal", a: "queue/42", b: "42", want: -1},
+		{name: "same legacy ID", a: "queue/42", b: "queue/42"},
+		{name: "queue containing slashes", a: "1", b: "request/monorepo/main/42", want: 1},
+		{name: "legacy queue containing slashes", a: "request/monorepo/main/9", b: "request/monorepo/main/10", want: -1},
+		{name: "maximum legacy counter is older than first decimal", a: "request/monorepo/main/9223372036854775807", b: "1", want: -1},
+		{name: "large legacy counters retain precision", a: "queue/9223372036854775806", b: "queue/9223372036854775807", want: -1},
+		{name: "invalid legacy suffix", a: "queue/bad", b: "queue/42", wantErr: true},
+		{name: "zero legacy suffix", a: "1", b: "queue/0", wantErr: true},
+		{name: "noncanonical legacy suffix", a: "queue/01", b: "queue/42", wantErr: true},
+		{name: "empty first legacy prefix", a: "/9", b: "10", wantErr: true},
+		{name: "empty second legacy prefix", a: "9", b: "/10", wantErr: true},
+		{name: "empty legacy suffix", a: "queue/", b: "10", wantErr: true},
+		{name: "overflow legacy suffix", a: "queue/9223372036854775808", b: "10", wantErr: true},
+		{name: "zero decimal against legacy", a: "0", b: "queue/42", wantErr: true},
+		{name: "noncanonical decimal against legacy", a: "queue/42", b: "01", wantErr: true},
+		{name: "negative decimal against legacy", a: "-1", b: "queue/42", wantErr: true},
+		{name: "empty decimal against legacy", a: "queue/42", wantErr: true},
+		{name: "overflow decimal against legacy", a: "9223372036854775808", b: "queue/42", wantErr: true},
 	}
 
 	for _, tt := range tests {

stovepipe/controller/ingest_test.go

@@ -184,6 +184,43 @@ func TestIngestController_Ingest(t *testing.T) {
 			},
 			wantID: "7",
 		},
+		{
+			name:  "new ID advances legacy latest pointer and publishes",
+			queue: testQueue,
+			setup: func(m ingestMocks) {
+				expectResolve(m)
+				m.uriStore.EXPECT().GetIDByURI(gomock.Any(), testURI).Return("", storage.ErrNotFound)
+				m.counter.EXPECT().Next(gomock.Any(), counterDomainRequest).Return(int64(1), nil)
+				m.uriStore.EXPECT().Create(gomock.Any(), testURI, "1").Return(nil)
+				m.reqStore.EXPECT().Get(gomock.Any(), "1").Return(entity.Request{}, storage.ErrNotFound)
+				m.reqStore.EXPECT().Create(gomock.Any(), acceptedRequest("1")).Return(nil)
+				expectMaterializeAccepted(m, "1")
+				m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{
+					Name: testQueue, LatestRequestID: "request/" + testQueue + "/42", Version: 1,
+				}, nil)
+				updated := entity.Queue{Name: testQueue, LatestRequestID: "1", Version: 1}
+				updateCall := m.queueStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil)
+				m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil).After(updateCall)
+			},
+			wantID: "1",
+		},
+		{
+			name:  "retry repairs accepted decimal request behind legacy pointer",
+			queue: testQueue,
+			setup: func(m ingestMocks) {
+				expectResolve(m)
+				m.uriStore.EXPECT().GetIDByURI(gomock.Any(), testURI).Return("1", nil)
+				m.reqStore.EXPECT().Get(gomock.Any(), "1").Return(acceptedRequest("1"), nil)
+				expectMaterializeAccepted(m, "1")
+				m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(entity.Queue{
+					Name: testQueue, LatestRequestID: "request/" + testQueue + "/42", Version: 1,
+				}, nil)
+				updated := entity.Queue{Name: testQueue, LatestRequestID: "1", Version: 1}
+				updateCall := m.queueStore.EXPECT().Update(gomock.Any(), updated, int32(1), int32(2)).Return(nil)
+				m.publisher.EXPECT().Publish(gomock.Any(), "process", gomock.Any()).Return(nil).After(updateCall)
+			},
+			wantID: "1",
+		},
 		{
 			name:  "dedup with existing accepted request republishes without minting",
 			queue: testQueue,

stovepipe/controller/record/record_test.go

@@ -291,6 +291,11 @@ func TestProcess_AdvancesBookmarkOnSuccess(t *testing.T) {
 			stored:  queueRow("git://remote/monorepo/main/old", "3", 4),
 			wantURI: testURI,
 		},
+		{
+			name:    "stored legacy bookmark is older",
+			stored:  queueRow("git://remote/monorepo/main/old", "request/"+testQueue+"/42", 4),
+			wantURI: testURI,
+		},
 	}
 
 	for _, tt := range tests {

stovepipe/entity/request_id.go

@@ -17,8 +17,10 @@ package entity
 import "github.com/uber/submitqueue/platform/base/id"
 
 // CompareRequestID compares ingest order of two request IDs in the same queue.
+// Callers must ensure the IDs belong to the same queue.
 // Returns -1 if a is older than b, 0 if equal, 1 if a is newer than b.
-// IDs are canonical decimal strings whose scope is carried separately.
+// Decimal IDs are newer than all legacy request/<queue>/<counter> IDs.
+// Within each format, ordering is by numeric counter.
 func CompareRequestID(a, b string) (int, error) {
 	return id.Compare(a, b)
 }