fix(github): align codebase to use a consistent github uri

entity/change_record.go

@@ -20,7 +20,7 @@ package entity
 // mergeability) becomes available.
 type ChangeRecord struct {
 	// URI identifies the change (RFC 3986). Same scheme/format as entity.Change.URIs.
-	// Example: "github://uber/submitqueue/pull/123/abc123def".
+	// Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab".
 	URI string `json:"uri"`
 
 	// RequestID is the owning land request that claimed this URI.

entity/github/change_id.go

@@ -21,12 +21,25 @@ import (
 )
 
 // changeIDFormat is the expected format for change IDs, included in error messages.
-const changeIDFormat = "{scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}"
+const changeIDFormat = "{scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}"
+
+// pullSegment is the literal segment separating the repo path from the pull
+// request number. Mirrors the path layout of an actual GitHub PR URL
+// (e.g. https://github.com/uber/repo/pull/123) so URIs can be constructed by
+// trivial substitution rather than reshaping.
+const pullSegment = "pull"
+
+// shaLength is the length of a GitHub commit SHA. GitHub's GraphQL API returns
+// full 40-char lowercase hex SHA-1 hashes via headRefOid, and the staleness
+// check compares the URI's SHA against that value with strict string equality,
+// so anything shorter or otherwise non-canonical will always fail later.
+// Validate up-front to fail fast at the gateway with a clearer error.
+const shaLength = 40
 
 // ChangeID represents a parsed GitHub-family change identifier.
 // Covers GitHub.com, GitHub Enterprise (GHE), and GitHub Enterprise Server (GHES)
 // since they share the same pull request model.
-// Format: {scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}
+// Format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
 type ChangeID struct {
 	// Scheme captures the source variant (e.g., "github", "ghe", "ghes").
 	Scheme string
@@ -41,9 +54,10 @@ type ChangeID struct {
 }
 
 // ParseChangeID parses a raw change ID string into a ChangeID.
-// Expected format: {scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}
+// Expected format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
 // The parser works from the end: SHA (last), PR number (second-to-last),
-// and everything before is the repo path (split into owner and repo).
+// the literal "pull" segment (third-to-last), and everything before is the
+// repo path (split into owner and repo).
 func ParseChangeID(raw string) (ChangeID, error) {
 	// Split on "://" to get scheme and path
 	schemeSplit := strings.SplitN(raw, "://", 2)
@@ -60,18 +74,26 @@ func ParseChangeID(raw string) (ChangeID, error) {
 
 	// Split the path into segments and parse from the end.
 	segments := strings.Split(path, "/")
-	// Need at least 4 segments: {owner}/{repo}/{pr_number}/{sha}
-	if len(segments) < 4 {
-		return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pr/sha, got %d segments (expected format: %s)", raw, len(segments), changeIDFormat)
+	// Need at least 5 segments: {owner}/{repo}/pull/{pr_number}/{sha}
+	if len(segments) < 5 {
+		return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pull/pr/sha, got %d segments (expected format: %s)", raw, len(segments), changeIDFormat)
 	}
 
 	sha := segments[len(segments)-1]
 	prStr := segments[len(segments)-2]
-	repoSegments := segments[:len(segments)-2]
+	pullLiteral := segments[len(segments)-3]
+	repoSegments := segments[:len(segments)-3]
+
+	if pullLiteral != pullSegment {
+		return ChangeID{}, fmt.Errorf("invalid change ID %q: expected literal %q segment before PR number, got %q (expected format: %s)", raw, pullSegment, pullLiteral, changeIDFormat)
+	}
 
 	if sha == "" {
 		return ChangeID{}, fmt.Errorf("invalid change ID %q: empty head commit SHA (expected format: %s)", raw, changeIDFormat)
 	}
+	if !isFullHexSHA(sha) {
+		return ChangeID{}, fmt.Errorf("invalid change ID %q: head commit SHA %q must be %d lowercase hex characters (expected format: %s)", raw, sha, shaLength, changeIDFormat)
+	}
 
 	prNumber, err := strconv.Atoi(prStr)
 	if err != nil {
@@ -104,10 +126,24 @@ func ParseChangeID(raw string) (ChangeID, error) {
 
 // String returns the string representation of the change ID.
 func (c ChangeID) String() string {
-	return fmt.Sprintf("%s://%s/%s/%d/%s", c.Scheme, c.Org, c.Repo, c.PRNumber, c.HeadCommitSHA)
+	return fmt.Sprintf("%s://%s/%s/%s/%d/%s", c.Scheme, c.Org, c.Repo, pullSegment, c.PRNumber, c.HeadCommitSHA)
 }
 
 // OwnerRepo returns the "{org}/{repo}" string.
 func (c ChangeID) OwnerRepo() string {
 	return fmt.Sprintf("%s/%s", c.Org, c.Repo)
 }
+
+// isFullHexSHA reports whether s is exactly shaLength lowercase hex characters.
+func isFullHexSHA(s string) bool {
+	if len(s) != shaLength {
+		return false
+	}
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		if !(c >= '0' && c <= '9') && !(c >= 'a' && c <= 'f') {
+			return false
+		}
+	}
+	return true
+}

entity/github/change_id_test.go

@@ -21,6 +21,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// Sample 40-char lowercase hex SHAs used across the test cases.
+const (
+	sha1Full = "1111111111111111111111111111111111111111"
+	sha2Full = "2222222222222222222222222222222222222222"
+	shaAFull = "abcdef0123456789abcdef0123456789abcdef01"
+	shaBFull = "0123456789abcdef0123456789abcdef01234567"
+	shaCFull = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+)
+
 func TestParseChangeID(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -30,86 +39,116 @@ func TestParseChangeID(t *testing.T) {
 	}{
 		{
 			name: "valid github scheme",
-			raw:  "github://uber/submitqueue/123/abc123def",
+			raw:  "github://uber/submitqueue/pull/123/" + shaAFull,
 			want: ChangeID{
 				Scheme:        "github",
 				Org:           "uber",
 				Repo:          "submitqueue",
 				PRNumber:      123,
-				HeadCommitSHA: "abc123def",
+				HeadCommitSHA: shaAFull,
 			},
 		},
 		{
 			name: "valid ghe scheme",
-			raw:  "ghe://uber/monorepo/456/deadbeef",
+			raw:  "ghe://uber/monorepo/pull/456/" + shaCFull,
 			want: ChangeID{
 				Scheme:        "ghe",
 				Org:           "uber",
 				Repo:          "monorepo",
 				PRNumber:      456,
-				HeadCommitSHA: "deadbeef",
+				HeadCommitSHA: shaCFull,
 			},
 		},
 		{
 			name: "valid ghes scheme",
-			raw:  "ghes://org/repo/1/sha1",
+			raw:  "ghes://org/repo/pull/1/" + sha1Full,
 			want: ChangeID{
 				Scheme:        "ghes",
 				Org:           "org",
 				Repo:          "repo",
 				PRNumber:      1,
-				HeadCommitSHA: "sha1",
+				HeadCommitSHA: sha1Full,
 			},
 		},
 		{
 			name: "nested org path",
-			raw:  "github://uber/frontend/webapp/42/abc123",
+			raw:  "github://uber/frontend/webapp/pull/42/" + shaAFull,
 			want: ChangeID{
 				Scheme:        "github",
 				Org:           "uber/frontend",
 				Repo:          "webapp",
 				PRNumber:      42,
-				HeadCommitSHA: "abc123",
+				HeadCommitSHA: shaAFull,
 			},
 		},
+		{
+			name:    "missing pull segment",
+			raw:     "github://uber/submitqueue/123/" + shaAFull,
+			wantErr: true,
+		},
+		{
+			name:    "wrong literal segment (issues instead of pull)",
+			raw:     "github://uber/submitqueue/issues/123/" + shaAFull,
+			wantErr: true,
+		},
 		{
 			name:    "missing separator",
-			raw:     "github/uber/submitqueue/123/abc123",
+			raw:     "github/uber/submitqueue/pull/123/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "empty scheme",
-			raw:     "://uber/submitqueue/123/abc123",
+			raw:     "://uber/submitqueue/pull/123/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "too few segments",
-			raw:     "github://uber/123/abc123",
+			raw:     "github://uber/pull/123/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "only one segment",
-			raw:     "github://abc123",
+			raw:     "github://" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "empty owner",
-			raw:     "github:///submitqueue/123/abc123",
+			raw:     "github:///submitqueue/pull/123/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "empty repo",
-			raw:     "github://uber//123/abc123",
+			raw:     "github://uber//pull/123/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "non-numeric PR number",
-			raw:     "github://uber/submitqueue/abc/abc123",
+			raw:     "github://uber/submitqueue/pull/abc/" + shaAFull,
 			wantErr: true,
 		},
 		{
 			name:    "empty SHA",
-			raw:     "github://uber/submitqueue/123/",
+			raw:     "github://uber/submitqueue/pull/123/",
+			wantErr: true,
+		},
+		{
+			name:    "abbreviated SHA",
+			raw:     "github://uber/submitqueue/pull/123/abc123def",
+			wantErr: true,
+		},
+		{
+			name:    "uppercase SHA",
+			raw:     "github://uber/submitqueue/pull/123/ABCDEF0123456789ABCDEF0123456789ABCDEF01",
+			wantErr: true,
+		},
+		{
+			name:    "non-hex SHA",
+			raw:     "github://uber/submitqueue/pull/123/zzzzzz0123456789abcdef0123456789abcdef01",
+			wantErr: true,
+		},
+		{
+			name:    "SHA too long",
+			raw:     "github://uber/submitqueue/pull/123/" + shaAFull + "ab",
 			wantErr: true,
 		},
 		{
@@ -145,9 +184,9 @@ func TestChangeID_String(t *testing.T) {
 				Org:           "uber",
 				Repo:          "submitqueue",
 				PRNumber:      123,
-				HeadCommitSHA: "abc123",
+				HeadCommitSHA: shaAFull,
 			},
-			want: "github://uber/submitqueue/123/abc123",
+			want: "github://uber/submitqueue/pull/123/" + shaAFull,
 		},
 		{
 			name: "ghe",
@@ -156,9 +195,9 @@ func TestChangeID_String(t *testing.T) {
 				Org:           "corp",
 				Repo:          "app",
 				PRNumber:      99,
-				HeadCommitSHA: "deadbeef",
+				HeadCommitSHA: shaCFull,
 			},
-			want: "ghe://corp/app/99/deadbeef",
+			want: "ghe://corp/app/pull/99/" + shaCFull,
 		},
 		{
 			name: "ghes",
@@ -167,9 +206,9 @@ func TestChangeID_String(t *testing.T) {
 				Org:           "org",
 				Repo:          "repo",
 				PRNumber:      1,
-				HeadCommitSHA: "sha1",
+				HeadCommitSHA: sha1Full,
 			},
-			want: "ghes://org/repo/1/sha1",
+			want: "ghes://org/repo/pull/1/" + sha1Full,
 		},
 	}
 
@@ -186,16 +225,17 @@ func TestChangeID_OwnerRepo(t *testing.T) {
 		Org:           "uber",
 		Repo:          "submitqueue",
 		PRNumber:      1,
-		HeadCommitSHA: "abc",
+		HeadCommitSHA: shaAFull,
 	}
 	assert.Equal(t, "uber/submitqueue", id.OwnerRepo())
 }
 
 func TestParseChangeID_RoundTrip(t *testing.T) {
 	originals := []string{
-		"github://uber/submitqueue/123/abc123def456",
-		"ghe://corp/monorepo/99/deadbeef01234567",
-		"ghes://org/repo/1/a1b2c3",
+		"github://uber/submitqueue/pull/123/" + shaAFull,
+		"ghe://corp/monorepo/pull/99/" + shaCFull,
+		"ghes://org/repo/pull/1/" + sha2Full,
+		"github://uber/frontend/webapp/pull/42/" + shaBFull,
 	}
 
 	for _, raw := range originals {

entity/land_request_test.go

@@ -23,9 +23,12 @@ import (
 
 func TestLandRequest_ToBytes(t *testing.T) {
 	req := LandRequest{
-		ID:           "test-queue/123",
-		Queue:        "test-queue",
-		Change:       Change{URIs: []string{"github://uber/submitqueue/pull/456/abc123def", "github://uber/submitqueue/pull/789/def456abc"}},
+		ID:    "test-queue/123",
+		Queue: "test-queue",
+		Change: Change{URIs: []string{
+			"github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01",
+			"github://uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567",
+		}},
 		LandStrategy: RequestLandStrategyRebase,
 	}
 
@@ -36,7 +39,7 @@ func TestLandRequest_ToBytes(t *testing.T) {
 	// Verify JSON contains expected fields
 	jsonStr := string(data)
 	assert.Contains(t, jsonStr, "test-queue/123")
-	assert.Contains(t, jsonStr, "github://uber/submitqueue/pull/456/abc123def")
+	assert.Contains(t, jsonStr, "github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01")
 	assert.Contains(t, jsonStr, "rebase")
 }
 
@@ -90,9 +93,13 @@ func TestLandRequest_SerializationRoundTrip(t *testing.T) {
 		{
 			name: "github stacked diff",
 			req: LandRequest{
-				ID:           "queue1/100",
-				Queue:        "queue1",
-				Change:       Change{URIs: []string{"github://uber/repo-a/pull/101/aaa111", "github://uber/repo-a/pull/102/bbb222", "github://uber/repo-a/pull/103/ccc333"}},
+				ID:    "queue1/100",
+				Queue: "queue1",
+				Change: Change{URIs: []string{
+					"github://uber/repo-a/pull/101/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+					"github://uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+					"github://uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc",
+				}},
 				LandStrategy: RequestLandStrategySquashRebase,
 			},
 		},

entity/request.go

@@ -60,8 +60,9 @@ type Change struct {
 	// The scheme identifies the change provider, and the path contains provider-specific resource identifiers.
 	//
 	// GitHub is supported by default (though other providers can be added):
-	//   Template: "github://<org>/<repo>/pull/<pr>/<hash>"
-	//   Example: "github://uber/submitqueue/pull/123/abc123def"
+	//   Template: "<scheme>://<org>/<repo>/pull/<pr>/<head_commit_sha>"
+	//   Example:  "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"
+	//   Schemes:  "github", "ghe", "ghes". Head commit SHA must be full 40-char lowercase hex.
 	//
 	URIs []string `json:"uris"`
 }