diff --git a/internal/gateway/github/async_merge.go b/internal/gateway/github/async_merge.go new file mode 100644 index 00000000..f3a8c14c --- /dev/null +++ b/internal/gateway/github/async_merge.go @@ -0,0 +1,193 @@ +package github + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strconv" + "strings" +) + +// MergePullRequestAsyncInput specifies one asynchronous pull request merge. +type MergePullRequestAsyncInput struct { + // Owner is the login that owns the repository. + Owner string // required + + // Repo is the repository name. + Repo string // required + + // PullRequestNumber identifies the pull request to merge. + // When the pull request belongs to a stack, GitHub merges the open stack + // prefix ending at this pull request. + PullRequestNumber int // required + + // ExpectedHeadSHA, when non-empty, requires the pull request head to match + // before merging. + ExpectedHeadSHA string + + // Method selects the merge strategy. + // The unknown value lets GitHub select the repository default. + Method MergeMethod +} + +// AsyncMergeResult describes the current state of an asynchronous merge. +type AsyncMergeResult struct { + // Status is GitHub's current disposition of the request. + Status AsyncMergeStatus + + // Message is GitHub's human-readable description, when provided. + Message string + + // OperationID identifies a pending operation for later status probes. + // It is non-empty when Status is [AsyncMergeStatusPending]. + OperationID string +} + +// MergePullRequestAsync submits one asynchronous pull request merge. +// GitHub's failed and already-pending responses are returned as results because +// those responses use the same protocol as successful submissions. +// See https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request-asynchronously. +func (c *Gateway) MergePullRequestAsync( + ctx context.Context, + input *MergePullRequestAsyncInput, +) (*AsyncMergeResult, error) { + var mergeMethod string + if input.Method != MergeMethodUnknown { + method, err := input.Method.MarshalText() + if err != nil { + return nil, fmt.Errorf("encode merge method: %w", err) + } + mergeMethod = strings.ToLower(string(method)) + } + req := struct { + ExpectedHeadSHA string `json:"sha,omitempty"` + MergeMethod string `json:"merge_method,omitempty"` + MergeAction string `json:"merge_action"` + }{ + ExpectedHeadSHA: input.ExpectedHeadSHA, + MergeMethod: mergeMethod, + MergeAction: "default", + } + + path := []string{ + "repos", + input.Owner, + input.Repo, + "pulls", + strconv.Itoa(input.PullRequestNumber), + "merge-async", + } + var res struct { + Status AsyncMergeStatus `json:"status"` + Details struct { + Message string `json:"message"` + UUID string `json:"uuid"` + } `json:"details"` + } + err := c.putREST( + ctx, + path, + &req, + &res, + http.StatusBadRequest, + http.StatusConflict, + ) + if err != nil { + return nil, fmt.Errorf("submit asynchronous merge: %w", err) + } + result := &AsyncMergeResult{ + Status: res.Status, + Message: res.Details.Message, + OperationID: res.Details.UUID, + } + if result.Status == AsyncMergeStatusPending && result.OperationID == "" { + return nil, errors.New("pending asynchronous merge has no operation ID") + } + return result, nil +} + +// AsyncMergeResult fetches the current state of one asynchronous merge. +// It performs one request and does not poll or wait. +// See https://docs.github.com/en/rest/pulls/pulls#get-the-result-of-an-asynchronous-merge. +func (c *Gateway) AsyncMergeResult( + ctx context.Context, + owner string, + repo string, + pullRequestNumber int, + operationID string, +) (*AsyncMergeResult, error) { + var res struct { + Status AsyncMergeStatus `json:"status"` + Details struct { + Message string `json:"message"` + UUID string `json:"uuid"` + } `json:"details"` + } + path := []string{ + "repos", + owner, + repo, + "pulls", + strconv.Itoa(pullRequestNumber), + "merge-async", + operationID, + } + if err := c.getREST(ctx, path, &res); err != nil { + return nil, fmt.Errorf("get asynchronous merge result: %w", err) + } + + result := &AsyncMergeResult{ + Status: res.Status, + Message: res.Details.Message, + OperationID: res.Details.UUID, + } + if result.Status == AsyncMergeStatusPending && result.OperationID == "" { + return nil, errors.New("pending asynchronous merge has no operation ID") + } + return result, nil +} + +// AsyncMergeStatus is GitHub's disposition of one asynchronous merge request. +type AsyncMergeStatus int + +const ( + // AsyncMergeStatusUnknown is the zero value and is not returned by GitHub. + AsyncMergeStatusUnknown AsyncMergeStatus = iota + + // AsyncMergeStatusPending means GitHub is still processing the request. + AsyncMergeStatusPending + + // AsyncMergeStatusMerged means GitHub completed the merge. + AsyncMergeStatusMerged + + // AsyncMergeStatusEnqueued means GitHub accepted the request into a merge + // queue. + // Callers must observe pull request state for final completion. + AsyncMergeStatusEnqueued + + // AsyncMergeStatusFailed means GitHub rejected the merge request. + AsyncMergeStatusFailed +) + +// UnmarshalJSON decodes the status strings returned by the async merge API. +func (s *AsyncMergeStatus) UnmarshalJSON(data []byte) error { + var status string + if err := json.Unmarshal(data, &status); err != nil { + return fmt.Errorf("decode asynchronous merge status: %w", err) + } + switch status { + case "pending": + *s = AsyncMergeStatusPending + case "merged": + *s = AsyncMergeStatusMerged + case "enqueued": + *s = AsyncMergeStatusEnqueued + case "failed": + *s = AsyncMergeStatusFailed + default: + return fmt.Errorf("unknown asynchronous merge status %q", status) + } + return nil +} diff --git a/internal/gateway/github/async_merge_test.go b/internal/gateway/github/async_merge_test.go new file mode 100644 index 00000000..24a55f96 --- /dev/null +++ b/internal/gateway/github/async_merge_test.go @@ -0,0 +1,243 @@ +package github + +import ( + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGateway_MergePullRequestAsync(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodPut, r.Method) + assert.Equal(t, "/repos/octo/hello/pulls/102/merge-async", r.URL.Path) + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + assert.JSONEq(t, `{ + "sha":"abc123", + "merge_method":"squash", + "merge_action":"default" + }`, string(body)) + return restJSONResponse(http.StatusAccepted, `{ + "status":"pending", + "details":{ + "message":"Merge request accepted.", + "uuid":"merge-uuid", + "merge_method":"squash", + "merge_action":"default", + "expected_head_sha":"abc123" + } + }`), nil + })) + + result, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + ExpectedHeadSHA: "abc123", + Method: MergeMethodSquash, + }) + require.NoError(t, err) + assert.Equal(t, AsyncMergeStatusPending, result.Status) + assert.Equal(t, "Merge request accepted.", result.Message) + assert.Equal(t, "merge-uuid", result.OperationID) +} + +func TestGateway_MergePullRequestAsyncStatuses(t *testing.T) { + tests := []struct { + name string + statusCode int + body string + wantStatus AsyncMergeStatus + wantID string + }{ + { + name: "Pending", + statusCode: http.StatusAccepted, + body: `{"status":"pending","details":{"message":"running","uuid":"new-uuid","merge_action":"default"}}`, + wantStatus: AsyncMergeStatusPending, + wantID: "new-uuid", + }, + { + name: "Merged", + statusCode: http.StatusOK, + body: `{"status":"merged","details":{"message":"merged","sha":"deadbeef"}}`, + wantStatus: AsyncMergeStatusMerged, + }, + { + name: "Enqueued", + statusCode: http.StatusOK, + body: `{"status":"enqueued","details":{"message":"queued"}}`, + wantStatus: AsyncMergeStatusEnqueued, + }, + { + name: "Failed", + statusCode: http.StatusBadRequest, + body: `{"status":"failed","details":{"message":"pull request is closed"}}`, + wantStatus: AsyncMergeStatusFailed, + }, + { + name: "ExistingPendingWithDifferentOptions", + statusCode: http.StatusConflict, + body: `{"status":"pending","details":{"message":"already running","uuid":"existing-uuid","expected_head_sha":"other","merge_method":"rebase","merge_action":"direct_merge"}}`, + wantStatus: AsyncMergeStatusPending, + wantID: "existing-uuid", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse(tt.statusCode, tt.body), nil + })) + + result, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + ExpectedHeadSHA: "abc123", + Method: MergeMethodSquash, + }) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, result.Status) + assert.Equal(t, tt.wantID, result.OperationID) + }) + } +} + +func TestGateway_MergePullRequestAsyncErrors(t *testing.T) { + for _, tt := range []struct { + name string + statusCode int + want error + }{ + {name: "NotFound", statusCode: http.StatusNotFound, want: ErrNotFound}, + {name: "Validation", statusCode: http.StatusUnprocessableEntity, want: ErrUnprocessable}, + } { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse(tt.statusCode, `{"message":"request rejected"}`), nil + })) + + _, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + }) + assert.ErrorIs(t, err, tt.want) + }) + } +} + +func TestGateway_MergePullRequestAsyncInvalidResult(t *testing.T) { + t.Run("Status", func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse( + http.StatusAccepted, + `{"status":"waiting","details":{"uuid":"merge-uuid"}}`, + ), nil + })) + + _, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + }) + assert.ErrorContains(t, err, `unknown asynchronous merge status "waiting"`) + }) + + t.Run("PendingWithoutOperationID", func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse( + http.StatusAccepted, + `{"status":"pending","details":{"message":"running"}}`, + ), nil + })) + + _, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + }) + assert.ErrorContains(t, err, "pending asynchronous merge has no operation ID") + }) + + t.Run("MergeMethod", func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("HTTP request made with invalid merge method") + return nil, nil + })) + + _, err := gateway.MergePullRequestAsync(t.Context(), &MergePullRequestAsyncInput{ + Owner: "octo", + Repo: "hello", + PullRequestNumber: 102, + Method: MergeMethod(100), + }) + assert.ErrorContains(t, err, "encode merge method: unknown GitHub enum value 100") + }) +} + +func TestGateway_AsyncMergeResult(t *testing.T) { + for _, tt := range []struct { + name string + body string + wantStatus AsyncMergeStatus + }{ + { + name: "Pending", + body: `{"status":"pending","details":{"message":"running","uuid":"merge-uuid"}}`, + wantStatus: AsyncMergeStatusPending, + }, + { + name: "Merged", + body: `{"status":"merged","details":{"message":"merged","sha":"deadbeef"}}`, + wantStatus: AsyncMergeStatusMerged, + }, + { + name: "Enqueued", + body: `{"status":"enqueued","details":{"message":"queued"}}`, + wantStatus: AsyncMergeStatusEnqueued, + }, + { + name: "Failed", + body: `{"status":"failed","details":{"message":"merge conflict"}}`, + wantStatus: AsyncMergeStatusFailed, + }, + } { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/repos/octo/hello/pulls/102/merge-async/merge-uuid", r.URL.Path) + assert.Empty(t, r.Header.Get("Content-Type")) + return restJSONResponse(http.StatusOK, tt.body), nil + })) + + result, err := gateway.AsyncMergeResult( + t.Context(), + "octo", + "hello", + 102, + "merge-uuid", + ) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, result.Status) + }) + } +} + +func TestGateway_AsyncMergeResultNotFound(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse(http.StatusNotFound, `{"message":"Not Found"}`), nil + })) + + _, err := gateway.AsyncMergeResult( + t.Context(), + "octo", + "hello", + 102, + "expired-uuid", + ) + assert.ErrorIs(t, err, ErrNotFound) +} diff --git a/internal/gateway/github/comment.go b/internal/gateway/github/comment.go index 0f941124..e29bd055 100644 --- a/internal/gateway/github/comment.go +++ b/internal/gateway/github/comment.go @@ -116,7 +116,7 @@ func (c *Gateway) pullRequestCommentsPage(ctx context.Context, id ID, first int, } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query pull request comments: %w", err) } comments := result.Node.Comments diff --git a/internal/gateway/github/error.go b/internal/gateway/github/error.go index 887e3aed..68409395 100644 --- a/internal/gateway/github/error.go +++ b/internal/gateway/github/error.go @@ -6,16 +6,16 @@ import ( "strings" ) -// Sentinel errors classify GitHub GraphQL error types. +// Sentinel errors classify GitHub errors. // Match them with [errors.Is]. var ( - // ErrNotFound matches a GraphQL error whose type is NOT_FOUND. + // ErrNotFound matches an error caused by a missing resource. ErrNotFound = errors.New("not found") - // ErrForbidden matches a GraphQL error whose type is FORBIDDEN. + // ErrForbidden matches an error caused by insufficient permission. ErrForbidden = errors.New("forbidden") - // ErrUnprocessable matches a GraphQL error whose type is UNPROCESSABLE. + // ErrUnprocessable matches an error caused by invalid input or state. ErrUnprocessable = errors.New("unprocessable") ) diff --git a/internal/gateway/github/gateway.go b/internal/gateway/github/gateway.go index d32e76a9..239cbc73 100644 --- a/internal/gateway/github/gateway.go +++ b/internal/gateway/github/gateway.go @@ -1,17 +1,15 @@ -// Package github provides the GitHub GraphQL operations needed by git-spice. +// Package github provides the typed GitHub API operations needed by git-spice. // -// The package owns GitHub's GraphQL wire protocol, authenticated request -// execution, and response error model. +// The package owns GitHub's GraphQL and REST wire protocols, +// authenticated request execution, +// and response error models. // Callers supply credentials through [TokenSource] and adapt the typed results // to their own domain models. // The package does not own credential discovery, persistence, or login flows. package github import ( - "bytes" "context" - "encoding/json/jsontext" - json "encoding/json/v2" "errors" "fmt" "io" @@ -20,7 +18,7 @@ import ( "strings" ) -// maxErrorBody limits the diagnostic content retained from a non-GraphQL HTTP +// maxErrorBody limits the diagnostic content retained from an HTTP error // response so a proxy or server cannot turn an error into an unbounded // allocation or diagnostic. const maxErrorBody = 4 * 1024 @@ -31,18 +29,22 @@ type TokenSource interface { Token(context.Context) (string, error) } -// Gateway executes the typed GitHub GraphQL operations exposed by this package. +// Gateway executes the typed GitHub operations exposed by this package. // // A Gateway is safe for concurrent use when its HTTP client and token source are // safe for concurrent use. -// Each operation retrieves a token with the operation context, sends one -// authenticated request to the configured GitHub endpoint, and decodes either -// its complete result or the GraphQL errors. +// Each HTTP request retrieves a token with the operation context and uses the +// configured GitHub endpoint. +// A typed operation may issue multiple requests when GitHub exposes related +// data through separate API nodes. // GitHub responses containing both data and errors return only the errors; // callers never observe a partial result. type Gateway struct { - // endpoint is the GraphQL endpoint derived from the configured API base URL. - endpoint string + // graphQLEndpoint is derived from the configured API base URL. + graphQLEndpoint string + + // restBaseURL is the REST API root derived from the configured API base URL. + restBaseURL *url.URL // httpClient performs requests after the gateway has supplied authentication. httpClient *http.Client @@ -53,125 +55,61 @@ type Gateway struct { // NewGateway builds a client for a GitHub API base URL. // -// apiURL is the REST API base URL reported by the forge configuration; -// NewGateway appends GitHub's GraphQL endpoint path. +// apiURLStr is the API base URL reported by the forge configuration. +// For GitHub Enterprise Server, this is the common `/api` root from which +// NewGateway derives `/api/graphql` and `/api/v3`. // A nil HTTP client uses [http.DefaultClient]. -// tokens is required and is consulted once for each operation. -func NewGateway(apiURL string, httpClient *http.Client, tokens TokenSource) (*Gateway, error) { - endpoint, err := url.JoinPath(apiURL, "/graphql") +// The tokens source is required and is consulted for every HTTP request. +func NewGateway(apiURLStr string, httpClient *http.Client, tokens TokenSource) (*Gateway, error) { + apiURL, err := url.Parse(apiURLStr) if err != nil { - return nil, fmt.Errorf("build GraphQL API URL: %w", err) + return nil, fmt.Errorf("parse API URL: %w", err) + } + + graphQLURL := apiURL.JoinPath("graphql") + + // TODO: Accept complete GraphQL and REST endpoint URLs instead of deriving + // both endpoints from GitHub's common API base URL. + // GitHub.com serves GraphQL below the REST API host, + // while GitHub Enterprise Server uses /api/graphql and /api/v3. + restBaseURL := apiURL + if !strings.EqualFold(apiURL.Hostname(), "api.github.com") { + restBaseURL = apiURL.JoinPath("v3") } + if httpClient == nil { httpClient = http.DefaultClient } if tokens == nil { return nil, errors.New("token source is required") } + return &Gateway{ - endpoint: endpoint, - httpClient: httpClient, - tokens: tokens, + graphQLEndpoint: graphQLURL.String(), + restBaseURL: restBaseURL, + httpClient: httpClient, + tokens: tokens, }, nil } -// graphQLRequestEnvelope is the JSON request shape accepted by GitHub's GraphQL API. -// Variables remains operation-specific so typed operations can own their wire -// fields without exposing generic GraphQL execution to callers. -type graphQLRequestEnvelope struct { - Query string `json:"query"` - Variables any `json:"variables"` -} - -// graphQLResponseEnvelope separates the GraphQL operation result from protocol-level -// errors before operation-specific decoding. -// Keeping Data raw ensures errors take precedence over partial data. -type graphQLResponseEnvelope struct { - Data jsontext.Value `json:"data"` - Errors graphQLError `json:"errors"` -} - -// execute performs the shared GraphQL request lifecycle for typed operations. -// -// query contains a GraphQL query or mutation. Query and variables must -// describe the operation's stable wire request, and -// result must be a pointer suitable for decoding the operation's data shape. -// execute retrieves credentials for every call so dynamic token sources retain -// their request lifetime. -// It owns response-body closure and converts transport, HTTP, protocol, and -// data-shape failures into errors with stage-specific context. -// A non-empty GraphQL error list always wins over Data to preserve the client's -// all-or-error result contract. -func (c *Gateway) execute(ctx context.Context, query string, variables, result any) error { - var body bytes.Buffer - if err := json.MarshalWrite( - &body, - graphQLRequestEnvelope{ - Query: query, - Variables: variables, - }, - json.Deterministic(true), - ); err != nil { - return fmt.Errorf("encode GraphQL request: %w", err) - } - +// newRequest builds one authenticated GitHub request. +// Token lookup stays at request lifetime rather than operation lifetime because +// one typed operation may issue multiple HTTP requests. +func (c *Gateway) newRequest( + ctx context.Context, + method string, + endpoint string, + body io.Reader, +) (*http.Request, error) { token, err := c.tokens.Token(ctx) if err != nil { - return fmt.Errorf("get GitHub token: %w", err) + return nil, fmt.Errorf("get GitHub token: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.endpoint, &body) + req, err := http.NewRequestWithContext(ctx, method, endpoint, body) if err != nil { - return fmt.Errorf("build GraphQL request: %w", err) + return nil, fmt.Errorf("build GitHub request: %w", err) } req.Header.Set("Authorization", "Bearer "+token) - req.Header.Set("Content-Type", "application/json") - - res, err := c.httpClient.Do(req) - if err != nil { - return fmt.Errorf("send GraphQL request: %w", err) - } - // GraphQL envelopes are defined only for successful HTTP responses. - // Bound diagnostics from other responses before closing the body so an - // intermediary cannot make error reporting consume unbounded memory. - if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices { - diagnostic, err := io.ReadAll(io.LimitReader(res.Body, maxErrorBody)) - err = errors.Join(err, res.Body.Close()) - if err != nil { - return fmt.Errorf( - "GitHub GraphQL HTTP status %s: read response: %w", - res.Status, - err, - ) - } - return fmt.Errorf( - "GitHub GraphQL HTTP status %s: %s", - res.Status, - strings.TrimSpace(string(diagnostic)), - ) - } - - responseBody, err := io.ReadAll(res.Body) - err = errors.Join(err, res.Body.Close()) - if err != nil { - return fmt.Errorf("read GraphQL response: %w", err) - } - - var envelope graphQLResponseEnvelope - if err := json.Unmarshal(responseBody, &envelope); err != nil { - return fmt.Errorf("decode GraphQL response: %w", err) - } - if len(envelope.Errors) > 0 { - return envelope.Errors - } - if err := json.Unmarshal(envelope.Data, result); err != nil { - return fmt.Errorf("decode GraphQL data: %w", err) - } - return nil -} - -func (c *Gateway) mutate(ctx context.Context, mutation string, input, result any) error { - return c.execute(ctx, mutation, struct { - Input any `json:"input"` - }{input}, result) + return req, nil } diff --git a/internal/gateway/github/gateway_test.go b/internal/gateway/github/gateway_test.go index 8286e947..4fd836c2 100644 --- a/internal/gateway/github/gateway_test.go +++ b/internal/gateway/github/gateway_test.go @@ -16,12 +16,23 @@ import ( func TestNewGateway_endpoint(t *testing.T) { tests := []struct { - name string - give string - want string + name string + give string + wantGQL string + wantREST string }{ - {name: "GitHub", give: "https://api.github.com", want: "https://api.github.com/graphql"}, - {name: "Enterprise", give: "https://github.example.com/api", want: "https://github.example.com/api/graphql"}, + { + name: "GitHub", + give: "https://api.github.com", + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "Enterprise", + give: "https://github.example.com/api", + wantGQL: "https://github.example.com/api/graphql", + wantREST: "https://github.example.com/api/v3", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -29,7 +40,8 @@ func TestNewGateway_endpoint(t *testing.T) { return "token", nil })) require.NoError(t, err) - assert.Equal(t, tt.want, gateway.endpoint) + assert.Equal(t, tt.wantGQL, gateway.graphQLEndpoint) + assert.Equal(t, tt.wantREST, gateway.restBaseURL.String()) }) } } diff --git a/internal/gateway/github/graphql.go b/internal/gateway/github/graphql.go index 3a2b9f34..62085073 100644 --- a/internal/gateway/github/graphql.go +++ b/internal/gateway/github/graphql.go @@ -1,6 +1,16 @@ package github -import "strings" +import ( + "bytes" + "context" + "encoding/json/jsontext" + json "encoding/json/v2" + "errors" + "fmt" + "io" + "net/http" + "strings" +) // compactGraphQL removes line indentation and line breaks from a GraphQL // document while preserving the contents of each trimmed line. @@ -11,3 +21,99 @@ func compactGraphQL(document string) string { } return query.String() } + +// graphQLRequestEnvelope is the JSON request shape accepted by GitHub's +// GraphQL API. +// Variables remains operation-specific so typed operations can own their wire +// fields without exposing generic GraphQL execution to callers. +type graphQLRequestEnvelope struct { + Query string `json:"query"` + Variables any `json:"variables"` +} + +// graphQLResponseEnvelope separates the GraphQL operation result from +// protocol-level errors before operation-specific decoding. +// Keeping Data raw ensures errors take precedence over partial data. +type graphQLResponseEnvelope struct { + Data jsontext.Value `json:"data"` + Errors graphQLError `json:"errors"` +} + +// executeGQL performs the shared GraphQL request lifecycle for typed operations. +// +// The query and variables describe the operation's stable wire request. +// The result must be a pointer suitable for decoding the operation's data +// shape. +// executeGQL retrieves credentials for every call so dynamic token sources +// retain their request lifetime. +// It owns response-body closure and converts transport, HTTP, protocol, and +// data-shape failures into errors with stage-specific context. +// A non-empty GraphQL error list always wins over Data to preserve the client's +// all-or-error result contract. +func (c *Gateway) executeGQL(ctx context.Context, query string, variables, result any) error { + var body bytes.Buffer + if err := json.MarshalWrite( + &body, + graphQLRequestEnvelope{ + Query: query, + Variables: variables, + }, + json.Deterministic(true), + ); err != nil { + return fmt.Errorf("encode GraphQL request: %w", err) + } + + req, err := c.newRequest(ctx, http.MethodPost, c.graphQLEndpoint, &body) + if err != nil { + return err + } + req.Header.Set("Content-Type", "application/json") + + res, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("send GraphQL request: %w", err) + } + // GraphQL envelopes are defined only for successful HTTP responses. + // Bound diagnostics from other responses before closing the body so an + // intermediary cannot make error reporting consume unbounded memory. + if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices { + diagnostic, err := io.ReadAll(io.LimitReader(res.Body, maxErrorBody)) + err = errors.Join(err, res.Body.Close()) + if err != nil { + return fmt.Errorf( + "GitHub GraphQL HTTP status %s: read response: %w", + res.Status, + err, + ) + } + return fmt.Errorf( + "GitHub GraphQL HTTP status %s: %s", + res.Status, + strings.TrimSpace(string(diagnostic)), + ) + } + + responseBody, err := io.ReadAll(res.Body) + err = errors.Join(err, res.Body.Close()) + if err != nil { + return fmt.Errorf("read GraphQL response: %w", err) + } + + var envelope graphQLResponseEnvelope + if err := json.Unmarshal(responseBody, &envelope); err != nil { + return fmt.Errorf("decode GraphQL response: %w", err) + } + if len(envelope.Errors) > 0 { + return envelope.Errors + } + if err := json.Unmarshal(envelope.Data, result); err != nil { + return fmt.Errorf("decode GraphQL data: %w", err) + } + return nil +} + +func (c *Gateway) mutate(ctx context.Context, mutation string, input, result any) error { + return c.executeGQL(ctx, mutation, struct { + Input any `json:"input"` + }{input}, result) +} diff --git a/internal/gateway/github/identity.go b/internal/gateway/github/identity.go index 1899b0a5..e15e647f 100644 --- a/internal/gateway/github/identity.go +++ b/internal/gateway/github/identity.go @@ -89,7 +89,7 @@ func (c *Gateway) IdentityIDs( ID ID `json:"id"` } `json:"team"` } - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, nil, fmt.Errorf("query identities: %w", err) } diff --git a/internal/gateway/github/label.go b/internal/gateway/github/label.go index a733ead9..da94a177 100644 --- a/internal/gateway/github/label.go +++ b/internal/gateway/github/label.go @@ -71,7 +71,7 @@ func (c *Gateway) LabelIDs(ctx context.Context, owner, repo string, labels []str ID ID `json:"id"` } `json:"repository"` } - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query labels: %w", err) } diff --git a/internal/gateway/github/pull_request_find.go b/internal/gateway/github/pull_request_find.go index 9ecaa51b..5b2385ca 100644 --- a/internal/gateway/github/pull_request_find.go +++ b/internal/gateway/github/pull_request_find.go @@ -105,7 +105,7 @@ func (c *Gateway) FindPullRequestsByBranches( Nodes []*PullRequestBranchMatch `json:"nodes"` } `json:"repository"` } - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query pull requests by branch: %w", err) } @@ -141,7 +141,7 @@ func (c *Gateway) FindPullRequests(ctx context.Context, owner, repo, branch stri } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query pull requests: %w", err) } return result.Repository.PullRequests.Nodes, nil @@ -166,7 +166,7 @@ func (c *Gateway) PullRequest(ctx context.Context, owner, repo string, number in } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query pull request: %w", err) } return result.Repository.PullRequest, nil diff --git a/internal/gateway/github/pull_request_find_test.go b/internal/gateway/github/pull_request_find_test.go index 43c0ebb2..ac14f01d 100644 --- a/internal/gateway/github/pull_request_find_test.go +++ b/internal/gateway/github/pull_request_find_test.go @@ -94,12 +94,28 @@ func TestGateway_FindPullRequests(t *testing.T) { } func TestGateway_PullRequest(t *testing.T) { - gateway := newResponseGateway(t, `{ - "data": {"repository": {"pullRequest": { - "id": "PR_1", "number": 1, "state": "OPEN" - }}} - }`) - got, err := gateway.PullRequest(t.Context(), "acme", "repo", 1) + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + var request struct { + Query string `json:"query"` + Variables struct { + Number int `json:"number"` + } `json:"variables"` + } + require.NoError(t, json.UnmarshalRead(r.Body, &request)) + assert.Equal(t, 2, request.Variables.Number) + assert.NotContains(t, request.Query, "stack") + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{ + "data": {"repository": {"pullRequest": { + "id": "PR_2", + "number": 2, + "state": "OPEN" + }}} + }`)), + }, nil + })) + got, err := gateway.PullRequest(t.Context(), "acme", "repo", 2) require.NoError(t, err) - assert.Equal(t, ID("PR_1"), got.ID) + assert.Equal(t, ID("PR_2"), got.ID) } diff --git a/internal/gateway/github/pull_request_id.go b/internal/gateway/github/pull_request_id.go index ad0fdf69..72d998ef 100644 --- a/internal/gateway/github/pull_request_id.go +++ b/internal/gateway/github/pull_request_id.go @@ -26,7 +26,7 @@ func (c *Gateway) PullRequestID(ctx context.Context, owner, repo string, number Owner string `json:"owner"` Repo string `json:"repo"` }{number, owner, repo} - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return "", fmt.Errorf("query pull request ID: %w", err) } return result.Repository.PullRequest.ID, nil diff --git a/internal/gateway/github/pull_request_merge_range.go b/internal/gateway/github/pull_request_merge_range.go new file mode 100644 index 00000000..bd6e65d4 --- /dev/null +++ b/internal/gateway/github/pull_request_merge_range.go @@ -0,0 +1,194 @@ +package github + +import ( + "context" + "fmt" + "strconv" + "strings" +) + +// MergeRangePullRequest is the compact pull request projection needed to +// validate an asynchronous range merge. +type MergeRangePullRequest struct { + // State is the pull request lifecycle state. + State PullRequestState + + // IsDraft reports whether the pull request is a draft. + IsDraft bool + + // BaseRefName is the target branch name. + BaseRefName string + + // HeadRefName is the source branch name. + HeadRefName string + + // HeadRefOID is the Git object ID at the head of the pull request. + HeadRefOID string + + // HeadRepositoryOwner is the login that owns the head repository. + HeadRepositoryOwner string + + // HeadRepositoryName is the head repository name. + HeadRepositoryName string + + // Stack is the native stack containing the pull request. + // It is nil when the pull request is not stacked or when its referenced + // stack stopped resolving before the follow-up query. + Stack *PullRequestStack +} + +// PullRequestsForMergeRange loads merge validation state and native-stack +// membership in input order. +// A nil result entry means that GitHub did not find the corresponding pull +// request. +// When Stack is non-nil, it includes all open members in base-up order. +// +// The result combines two API reads because GitHub exposes stack identity on +// the pull request and ordered membership on a separate stack node. +// If a referenced stack stops resolving between those requests, the pull +// request result remains present with Stack nil. +// +// GitHub exposes PullRequestStack as a GraphQL node, so this operation loads +// each unique stack once rather than repeating its entries for every member. +// See https://docs.github.com/en/graphql/reference/pulls#pullrequeststack. +func (c *Gateway) PullRequestsForMergeRange( + ctx context.Context, + owner string, + repo string, + numbers []int, +) ([]*MergeRangePullRequest, error) { + if len(numbers) == 0 { + return nil, nil + } + + variables := make(map[string]any, len(numbers)+2) + variables["owner"] = owner + variables["repo"] = repo + + // Build one aliased repository selection per pull request number: + // + // query($owner:String!$repo:String!$pr0:Int!$pr1:Int!) { + // repository(owner: $owner, name: $repo) { + // pr0: pullRequest(number: $pr0) { + // state + // isDraft + // baseRefName + // headRefName + // headRefOid + // headRepository { owner { login } name } + // stack { id } + // } + // pr1: pullRequest(number: $pr1) { + // state + // isDraft + // baseRefName + // headRefName + // headRefOid + // headRepository { owner { login } name } + // stack { id } + // } + // } + // } + var variableDefinitions strings.Builder + variableDefinitions.WriteString("$owner:String!,$repo:String!") + var selections strings.Builder + + // Indexed aliases preserve one response slot for every input number, + // including duplicate numbers and pull requests GitHub does not find. + for i, number := range numbers { + alias := "pr" + strconv.Itoa(i) + fmt.Fprintf(&variableDefinitions, ",$%s:Int!", alias) + if i > 0 { + selections.WriteByte(',') + } + fmt.Fprintf( + &selections, + "%[1]s:pullRequest(number: $%[1]s){state,isDraft,baseRefName,headRefName,headRefOid,headRepository{owner{login},name},stack{id}}", + alias, + ) + variables[alias] = number + } + + var result struct { + Repository map[string]*struct { + State PullRequestState `json:"state"` + IsDraft bool `json:"isDraft"` + BaseRefName string `json:"baseRefName"` + HeadRefName string `json:"headRefName"` + HeadRefOID string `json:"headRefOid"` + HeadRepository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + Name string `json:"name"` + } `json:"headRepository"` + Stack *struct { + ID ID `json:"id"` + } `json:"stack"` + } `json:"repository"` + } + query := compactGraphQL( + fmt.Sprintf(` + query(%s){ + repository(owner: $owner, name: $repo){%s} + } + `, variableDefinitions.String(), selections.String()), + ) + if err := c.executeGQL(ctx, query, variables, &result); err != nil { + return nil, fmt.Errorf("query pull requests for merge range: %w", err) + } + + pullRequests := make([]*MergeRangePullRequest, len(numbers)) + pullRequestsAwaitingStackByID := make(map[ID][]*MergeRangePullRequest) + var stackIDsToResolve []ID + for i := range numbers { + res := result.Repository["pr"+strconv.Itoa(i)] + if res == nil { + continue + } + + pullRequest := &MergeRangePullRequest{ + State: res.State, + IsDraft: res.IsDraft, + BaseRefName: res.BaseRefName, + HeadRefName: res.HeadRefName, + HeadRefOID: res.HeadRefOID, + HeadRepositoryOwner: res.HeadRepository.Owner.Login, + HeadRepositoryName: res.HeadRepository.Name, + } + pullRequests[i] = pullRequest + if res.Stack == nil { + continue + } + + stackID := res.Stack.ID + if _, seen := pullRequestsAwaitingStackByID[stackID]; !seen { + stackIDsToResolve = append(stackIDsToResolve, stackID) + } + pullRequestsAwaitingStackByID[stackID] = append( + pullRequestsAwaitingStackByID[stackID], + pullRequest, + ) + } + if len(stackIDsToResolve) == 0 { + return pullRequests, nil + } + + // Resolve the ordered open members for every unique stack ID in one query. + // A node may disappear after the pull request query; in that case, leaving + // Stack nil preserves the best remote view this non-atomic operation obtained. + resolvedStacksByID, err := c.pullRequestStacksByID(ctx, stackIDsToResolve) + if err != nil { + return nil, err + } + for stackID, awaitingPullRequests := range pullRequestsAwaitingStackByID { + resolvedStack, ok := resolvedStacksByID[stackID] + if !ok { + continue + } + for _, pullRequest := range awaitingPullRequests { + pullRequest.Stack = resolvedStack + } + } + return pullRequests, nil +} diff --git a/internal/gateway/github/pull_request_merge_range_test.go b/internal/gateway/github/pull_request_merge_range_test.go new file mode 100644 index 00000000..888df94d --- /dev/null +++ b/internal/gateway/github/pull_request_merge_range_test.go @@ -0,0 +1,164 @@ +package github + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGateway_PullRequestsForMergeRange(t *testing.T) { + requestNumber := 0 + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + requestNumber++ + var request struct { + Query string `json:"query"` + Variables json.RawMessage `json:"variables"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + + switch requestNumber { + case 1: + assert.Contains(t, request.Query, "pr0:pullRequest(number: $pr0)") + assert.Contains(t, request.Query, "pr2:pullRequest(number: $pr2)") + assert.Contains(t, request.Query, "state,isDraft,baseRefName,headRefName,headRefOid") + assert.Contains(t, request.Query, "headRepository{owner{login},name},stack{id}") + assert.NotContains(t, request.Query, "labels") + assert.NotContains(t, request.Query, "entries") + assert.JSONEq(t, `{ + "owner": "acme", + "repo": "repo", + "pr0": 1, + "pr1": 2, + "pr2": 3 + }`, string(request.Variables)) + return graphQLResponse(`{ + "data": {"repository": { + "pr0": { + "state": "OPEN", + "isDraft": false, + "baseRefName": "main", + "headRefName": "bottom", + "headRefOid": "hash-1", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }, + "pr1": { + "state": "OPEN", + "isDraft": true, + "baseRefName": "bottom", + "headRefName": "top", + "headRefOid": "hash-2", + "headRepository": { + "owner": {"login": "fork"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }, + "pr2": null + }} + }`), nil + + case 2: + assert.JSONEq(t, `{"ids":["STACK_42"]}`, string(request.Variables)) + return graphQLResponse(`{ + "data": {"nodes": [{ + "id": "STACK_42", + "number": 42, + "entries": {"nodes": [ + {"pullRequest": {"number": 1, "state": "OPEN"}}, + {"pullRequest": {"number": 2, "state": "OPEN"}}, + {"pullRequest": {"number": 4, "state": "MERGED"}} + ]} + }]} + }`), nil + + default: + t.Fatalf("unexpected request %d", requestNumber) + return nil, nil + } + })) + + got, err := gateway.PullRequestsForMergeRange( + t.Context(), + "acme", + "repo", + []int{1, 2, 3}, + ) + require.NoError(t, err) + require.Len(t, got, 3) + assert.Equal(t, PullRequestStateOpen, got[0].State) + assert.False(t, got[0].IsDraft) + assert.Equal(t, "main", got[0].BaseRefName) + assert.Equal(t, "bottom", got[0].HeadRefName) + assert.Equal(t, "hash-1", got[0].HeadRefOID) + assert.Equal(t, "acme", got[0].HeadRepositoryOwner) + assert.Equal(t, "repo", got[0].HeadRepositoryName) + require.NotNil(t, got[0].Stack) + assert.Equal(t, 42, got[0].Stack.Number) + assert.Equal(t, []PullRequestStackMember{ + {Number: 1}, + {Number: 2}, + }, got[0].Stack.Members) + assert.True(t, got[1].IsDraft) + assert.Equal(t, "fork", got[1].HeadRepositoryOwner) + assert.Same(t, got[0].Stack, got[1].Stack) + assert.Nil(t, got[2]) + assert.Equal(t, 2, requestNumber) +} + +func TestGateway_PullRequestsForMergeRange_stackStopsResolving(t *testing.T) { + requestNumber := 0 + gateway := newTestGateway(t, roundTripFunc(func(_ *http.Request) (*http.Response, error) { + requestNumber++ + switch requestNumber { + case 1: + return graphQLResponse(`{ + "data": {"repository": {"pr0": { + "state": "OPEN", + "isDraft": false, + "baseRefName": "main", + "headRefName": "feature", + "headRefOid": "hash-1", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }}} + }`), nil + case 2: + return graphQLResponse(`{"data":{"nodes":[null]}}`), nil + default: + t.Fatalf("unexpected request %d", requestNumber) + return nil, nil + } + })) + + got, err := gateway.PullRequestsForMergeRange( + t.Context(), + "acme", + "repo", + []int{1}, + ) + require.NoError(t, err) + require.Len(t, got, 1) + require.NotNil(t, got[0]) + assert.Nil(t, got[0].Stack) +} + +func TestGateway_PullRequestsForMergeRange_empty(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("unexpected request") + return nil, nil + })) + + got, err := gateway.PullRequestsForMergeRange(t.Context(), "acme", "repo", nil) + require.NoError(t, err) + assert.Nil(t, got) +} diff --git a/internal/gateway/github/pull_request_mergeability.go b/internal/gateway/github/pull_request_mergeability.go index 9458cf70..d11c6372 100644 --- a/internal/gateway/github/pull_request_mergeability.go +++ b/internal/gateway/github/pull_request_mergeability.go @@ -29,7 +29,7 @@ func (c *Gateway) PullRequestMergeability(ctx context.Context, id ID) (*Mergeabi } } `) - if err := c.execute(ctx, query, struct { + if err := c.executeGQL(ctx, query, struct { ID ID `json:"id"` }{id}, &result); err != nil { return nil, fmt.Errorf("query pull request mergeability: %w", err) diff --git a/internal/gateway/github/pull_request_metadata.go b/internal/gateway/github/pull_request_metadata.go index 29335cc1..aaa015c1 100644 --- a/internal/gateway/github/pull_request_metadata.go +++ b/internal/gateway/github/pull_request_metadata.go @@ -108,5 +108,5 @@ func (c *Gateway) AddPullRequestMetadata(ctx context.Context, input *PullRequest mutation := compactGraphQL( "mutation(" + variableDefinitions.String() + "){" + fields.String() + "}", ) - return c.execute(ctx, mutation, variables, &struct{}{}) + return c.executeGQL(ctx, mutation, variables, &struct{}{}) } diff --git a/internal/gateway/github/pull_request_stack_lookup_test.go b/internal/gateway/github/pull_request_stack_lookup_test.go new file mode 100644 index 00000000..9c1daeaf --- /dev/null +++ b/internal/gateway/github/pull_request_stack_lookup_test.go @@ -0,0 +1,261 @@ +package github + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGateway_PullRequestsForStackUpdate(t *testing.T) { + requestNumber := 0 + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + requestNumber++ + var request struct { + Query string `json:"query"` + Variables json.RawMessage `json:"variables"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + + switch requestNumber { + case 1: + assert.Contains(t, request.Query, "pr0:pullRequest(number: $pr0)") + assert.Contains(t, request.Query, "pr3:pullRequest(number: $pr3)") + assert.Contains(t, request.Query, "id,state,headRefName,baseRefName,headRepository{owner{login},name},stack{id}") + assert.NotContains(t, request.Query, "labels") + assert.NotContains(t, request.Query, "entries") + assert.JSONEq(t, `{ + "owner": "acme", + "repo": "repo", + "pr0": 1, + "pr1": 2, + "pr2": 3, + "pr3": 4 + }`, string(request.Variables)) + return graphQLResponse(`{ + "data": {"repository": { + "pr0": { + "id": "PR_1", + "state": "OPEN", + "headRefName": "bottom", + "baseRefName": "main", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }, + "pr1": { + "id": "PR_2", + "state": "OPEN", + "headRefName": "top", + "baseRefName": "bottom", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }, + "pr2": { + "state": "CLOSED", + "headRepository": { + "owner": {"login": "someone"}, + "name": "fork" + }, + "stack": null + }, + "pr3": null + }} + }`), nil + + case 2: + assert.Equal(t, + "query($ids:[ID!]!){nodes(ids: $ids){... on PullRequestStack{id,number,entries(first: 100){nodes{pullRequest{number,state,mergeQueueEntry{id},autoMergeRequest{enabledAt}}},pageInfo{endCursor,hasNextPage}}}}}", + request.Query, + ) + assert.JSONEq(t, `{"ids":["STACK_42"]}`, string(request.Variables)) + return graphQLResponse(`{ + "data": {"nodes": [{ + "id": "STACK_42", + "number": 42, + "entries": {"nodes": [ + {"pullRequest": {"number": 1, "state": "OPEN", "mergeQueueEntry": null, "autoMergeRequest": null}}, + {"pullRequest": {"number": 2, "state": "OPEN", "mergeQueueEntry": {"id":"QUEUE"}, "autoMergeRequest": null}}, + {"pullRequest": {"number": 5, "state": "MERGED"}} + ], "pageInfo": {"hasNextPage": false}} + }]} + }`), nil + + default: + t.Fatalf("unexpected request %d", requestNumber) + return nil, nil + } + })) + + got, err := gateway.PullRequestsForStackUpdate( + t.Context(), + "acme", + "repo", + []int{1, 2, 3, 4}, + ) + require.NoError(t, err) + require.Len(t, got, 4) + assert.Equal(t, ID("PR_1"), got[0].ID) + assert.Equal(t, PullRequestStateOpen, got[0].State) + assert.Equal(t, "bottom", got[0].HeadRefName) + assert.Equal(t, "main", got[0].BaseRefName) + assert.Equal(t, "acme", got[0].HeadRepositoryOwner) + assert.Equal(t, "repo", got[0].HeadRepositoryName) + require.NotNil(t, got[0].Stack) + assert.Equal(t, 42, got[0].Stack.Number) + assert.Equal(t, []PullRequestStackMember{ + {Number: 1}, + {Number: 2, Locked: true}, + }, got[0].Stack.Members) + require.NotNil(t, got[1].Stack) + assert.Equal(t, 42, got[1].Stack.Number) + assert.Same(t, got[0].Stack, got[1].Stack) + assert.Equal(t, PullRequestStateClosed, got[2].State) + assert.Equal(t, "someone", got[2].HeadRepositoryOwner) + assert.Equal(t, "fork", got[2].HeadRepositoryName) + assert.Nil(t, got[2].Stack) + assert.Nil(t, got[3]) + assert.Equal(t, 2, requestNumber) +} + +func TestGateway_PullRequestsForStackUpdate_paginatesStackEntries(t *testing.T) { + requestNumber := 0 + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + requestNumber++ + var request struct { + Query string `json:"query"` + Variables json.RawMessage `json:"variables"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + + switch requestNumber { + case 1: + return graphQLResponse(`{ + "data": {"repository": {"pr0": { + "state": "OPEN", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }}} + }`), nil + case 2: + assert.Contains(t, request.Query, "entries(first: 100)") + assert.Contains(t, request.Query, "pageInfo{endCursor,hasNextPage}") + return graphQLResponse(`{ + "data": {"nodes": [{ + "id": "STACK_42", + "number": 42, + "entries": { + "nodes": [ + {"pullRequest": {"number": 1, "state": "OPEN"}}, + {"pullRequest": {"number": 2, "state": "MERGED"}} + ], + "pageInfo": {"endCursor": "cursor-1", "hasNextPage": true} + } + }]} + }`), nil + case 3: + assert.Equal(t, + "query($after:String!$id:ID!){node(id: $id){... on PullRequestStack{entries(first: 100, after: $after){nodes{pullRequest{number,state,mergeQueueEntry{id},autoMergeRequest{enabledAt}}},pageInfo{endCursor,hasNextPage}}}}}", + request.Query, + ) + assert.JSONEq(t, + `{"after":"cursor-1","id":"STACK_42"}`, + string(request.Variables), + ) + return graphQLResponse(`{ + "data": {"node": {"entries": { + "nodes": [ + {"pullRequest": {"number": 3, "state": "OPEN", "autoMergeRequest": {"enabledAt": "2026-08-23T00:00:00Z"}}} + ], + "pageInfo": {"hasNextPage": false} + }}} + }`), nil + default: + t.Fatalf("unexpected request %d", requestNumber) + return nil, nil + } + })) + + got, err := gateway.PullRequestsForStackUpdate( + t.Context(), + "acme", + "repo", + []int{1}, + ) + require.NoError(t, err) + require.Len(t, got, 1) + require.NotNil(t, got[0].Stack) + assert.Equal(t, []PullRequestStackMember{ + {Number: 1}, + {Number: 3, Locked: true}, + }, got[0].Stack.Members) + assert.Equal(t, 3, requestNumber) +} + +func TestGateway_PullRequestsForStackUpdate_empty(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("unexpected request") + return nil, nil + })) + + got, err := gateway.PullRequestsForStackUpdate(t.Context(), "acme", "repo", nil) + require.NoError(t, err) + assert.Nil(t, got) +} + +func TestGateway_PullRequestsForStackUpdate_stackStopsResolving(t *testing.T) { + requestNumber := 0 + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + requestNumber++ + switch requestNumber { + case 1: + return graphQLResponse(`{ + "data": {"repository": {"pr0": { + "state": "OPEN", + "headRepository": { + "owner": {"login": "acme"}, + "name": "repo" + }, + "stack": {"id": "STACK_42"} + }}} + }`), nil + case 2: + return graphQLResponse(`{"data":{"nodes":[null]}}`), nil + default: + t.Fatalf("unexpected request %d", requestNumber) + return nil, nil + } + })) + + got, err := gateway.PullRequestsForStackUpdate( + t.Context(), + "acme", + "repo", + []int{1}, + ) + require.NoError(t, err) + require.Len(t, got, 1) + require.NotNil(t, got[0]) + assert.Nil(t, got[0].Stack) + assert.Equal(t, 2, requestNumber) +} + +func graphQLResponse(body string) *http.Response { + return &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Body: io.NopCloser(strings.NewReader(body)), + } +} diff --git a/internal/gateway/github/pull_request_status.go b/internal/gateway/github/pull_request_status.go index 169193bb..ec290436 100644 --- a/internal/gateway/github/pull_request_status.go +++ b/internal/gateway/github/pull_request_status.go @@ -26,7 +26,7 @@ func (c *Gateway) ChangeStatuses(ctx context.Context, ids []ID) ([]*ChangeStatus } } `) - if err := c.execute(ctx, query, struct { + if err := c.executeGQL(ctx, query, struct { IDs []ID `json:"ids"` }{ids}, &result); err != nil { return nil, fmt.Errorf("query change statuses: %w", err) diff --git a/internal/gateway/github/ref.go b/internal/gateway/github/ref.go index a6cf280d..0aab6263 100644 --- a/internal/gateway/github/ref.go +++ b/internal/gateway/github/ref.go @@ -26,7 +26,7 @@ func (c *Gateway) RefExists(ctx context.Context, owner, repo, ref string) (bool, } } `) - if err := c.execute(ctx, query, vars, &result); err != nil { + if err := c.executeGQL(ctx, query, vars, &result); err != nil { return false, fmt.Errorf("query ref: %w", err) } return result.Repository.Ref != nil, nil diff --git a/internal/gateway/github/repository.go b/internal/gateway/github/repository.go index 61a117d3..0d4082c9 100644 --- a/internal/gateway/github/repository.go +++ b/internal/gateway/github/repository.go @@ -19,7 +19,7 @@ func (c *Gateway) RepositoryID(ctx context.Context, owner, repo string) (ID, err ID ID `json:"id"` } `json:"repository"` } - if err := c.execute(ctx, query, struct { + if err := c.executeGQL(ctx, query, struct { Owner string `json:"owner"` Repo string `json:"repo"` }{Owner: owner, Repo: repo}, &result); err != nil { diff --git a/internal/gateway/github/rest.go b/internal/gateway/github/rest.go new file mode 100644 index 00000000..be32855c --- /dev/null +++ b/internal/gateway/github/rest.go @@ -0,0 +1,173 @@ +package github + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "slices" + "strings" +) + +const ( + restAPIVersion = "2022-11-28" + restMediaType = "application/vnd.github+json" +) + +// githubRESTError reports a non-successful GitHub REST response. +// Its diagnostic is bounded so error reporting cannot consume an unbounded +// response from GitHub or an intermediary. +type githubRESTError struct { + statusCode int + status string + diagnostic string +} + +func (e *githubRESTError) Error() string { + if e.diagnostic == "" { + return "GitHub REST HTTP status " + e.status + } + return fmt.Sprintf("GitHub REST HTTP status %s: %s", e.status, e.diagnostic) +} + +func (e *githubRESTError) Is(target error) bool { + switch target { + case ErrForbidden: + return e.statusCode == http.StatusForbidden + case ErrNotFound: + return e.statusCode == http.StatusNotFound + case ErrUnprocessable: + return e.statusCode == http.StatusUnprocessableEntity + default: + return false + } +} + +// getREST performs an authenticated GET request and, when res is non-nil, +// decodes its JSON response into res. +func (c *Gateway) getREST( + ctx context.Context, + path []string, + res any, +) error { + return c.doREST(ctx, http.MethodGet, path, nil, res) +} + +// postREST performs an authenticated POST request with req as its JSON body +// and, when res is non-nil, decodes the JSON response into res. +func (c *Gateway) postREST( + ctx context.Context, + path []string, + req any, + res any, +) error { + return c.doREST(ctx, http.MethodPost, path, req, res) +} + +// deleteREST performs an authenticated DELETE request and, when res is +// non-nil, decodes its JSON response into res. +func (c *Gateway) deleteREST( + ctx context.Context, + path []string, + req any, + res any, +) error { + return c.doREST(ctx, http.MethodDelete, path, req, res) +} + +// putREST performs an authenticated PUT request with req as its JSON body and, +// when res is non-nil, decodes the JSON response into res. +// acceptedStatuses identifies non-2xx responses whose bodies still use res's +// shape. +func (c *Gateway) putREST( + ctx context.Context, + path []string, + req any, + res any, + acceptedStatuses ...int, +) error { + return c.doREST( + ctx, + http.MethodPut, + path, + req, + res, + acceptedStatuses..., + ) +} + +// doREST owns the transport protocol shared by typed REST operations. +// Every 2xx response is successful. +// The acceptedStatuses parameter adds operation-specific non-2xx responses +// whose bodies still use result's wire shape. +func (c *Gateway) doREST( + ctx context.Context, + method string, + path []string, + input any, + result any, + acceptedStatuses ...int, +) error { + var requestBody io.Reader + if input != nil { + var body bytes.Buffer + if err := json.NewEncoder(&body).Encode(input); err != nil { + return fmt.Errorf("encode REST request: %w", err) + } + requestBody = &body + } + + endpoint := c.restBaseURL.JoinPath(path...) + req, err := c.newRequest(ctx, method, endpoint.String(), requestBody) + if err != nil { + return err + } + req.Header.Set("Accept", restMediaType) + req.Header.Set("X-GitHub-Api-Version", restAPIVersion) + if input != nil { + req.Header.Set("Content-Type", "application/json") + } + + res, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("send REST request: %w", err) + } + if (res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices) && + !slices.Contains(acceptedStatuses, res.StatusCode) { + return readRESTError(res) + } + + responseBody, err := io.ReadAll(res.Body) + err = errors.Join(err, res.Body.Close()) + if err != nil { + return fmt.Errorf("read REST response: %w", err) + } + if result == nil || len(responseBody) == 0 { + return nil + } + if err := json.Unmarshal(responseBody, result); err != nil { + return fmt.Errorf("decode REST response: %w", err) + } + return nil +} + +func readRESTError(res *http.Response) error { + diagnostic, err := io.ReadAll(io.LimitReader(res.Body, maxErrorBody)) + err = errors.Join(err, res.Body.Close()) + status := res.Status + if status == "" { + status = fmt.Sprintf("%d %s", res.StatusCode, http.StatusText(res.StatusCode)) + } + restErr := &githubRESTError{ + statusCode: res.StatusCode, + status: status, + diagnostic: strings.TrimSpace(string(diagnostic)), + } + if err == nil { + return restErr + } + return errors.Join(restErr, fmt.Errorf("read REST response: %w", err)) +} diff --git a/internal/gateway/github/rest_test.go b/internal/gateway/github/rest_test.go new file mode 100644 index 00000000..abd0cadd --- /dev/null +++ b/internal/gateway/github/rest_test.go @@ -0,0 +1,156 @@ +package github + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGateway_RESTAuthenticationRefresh(t *testing.T) { + var tokenCalls int + gateway, err := NewGateway( + "https://api.github.com", + &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, fmt.Sprintf("Bearer token-%d", tokenCalls), r.Header.Get("Authorization")) + return restJSONResponse(http.StatusCreated, `{}`), nil + })}, + tokenSourceFunc(func(ctx context.Context) (string, error) { + assert.Same(t, t.Context(), ctx) + tokenCalls++ + return fmt.Sprintf("token-%d", tokenCalls), nil + }), + ) + require.NoError(t, err) + + for range 2 { + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + require.NoError(t, err) + } + assert.Equal(t, 2, tokenCalls) +} + +func TestGateway_RESTTokenError(t *testing.T) { + want := errors.New("token unavailable") + gateway, err := NewGateway( + "https://api.github.com", + &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("HTTP request made after token failure") + return nil, nil + })}, + tokenSourceFunc(func(context.Context) (string, error) { + return "", want + }), + ) + require.NoError(t, err) + + err = gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + assert.ErrorIs(t, err, want) +} + +func TestGateway_RESTErrorClassification(t *testing.T) { + tests := []struct { + name string + statusCode int + want error + }{ + {name: "Forbidden", statusCode: http.StatusForbidden, want: ErrForbidden}, + {name: "NotFound", statusCode: http.StatusNotFound, want: ErrNotFound}, + {name: "Unprocessable", statusCode: http.StatusUnprocessableEntity, want: ErrUnprocessable}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse(tt.statusCode, `{"message":"request rejected"}`), nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + assert.ErrorIs(t, err, tt.want) + + var restErr *githubRESTError + require.ErrorAs(t, err, &restErr) + assert.Equal(t, tt.statusCode, restErr.statusCode) + assert.Contains(t, restErr.diagnostic, "request rejected") + }) + } +} + +func TestGateway_RESTErrorBoundedDiagnostic(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse( + http.StatusUnprocessableEntity, + strings.Repeat("x", maxErrorBody+100), + ), nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + var restErr *githubRESTError + require.ErrorAs(t, err, &restErr) + assert.Len(t, restErr.diagnostic, maxErrorBody) + assert.Less(t, len(err.Error()), maxErrorBody+250) +} + +func TestGateway_RESTErrorReadFailurePreservesClassification(t *testing.T) { + want := errors.New("read failure") + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusNotFound, + Body: errorReader{err: want}, + }, nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + assert.ErrorIs(t, err, ErrNotFound) + assert.ErrorIs(t, err, want) +} + +func TestGateway_RESTResponseReadFailure(t *testing.T) { + want := errors.New("read failure") + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusCreated, + Body: errorReader{err: want}, + }, nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + assert.ErrorIs(t, err, want) +} + +func restJSONResponse(statusCode int, body string) *http.Response { + return &http.Response{ + StatusCode: statusCode, + Body: io.NopCloser(strings.NewReader(body)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + } +} diff --git a/internal/gateway/github/review.go b/internal/gateway/github/review.go index d12b14d7..326665a3 100644 --- a/internal/gateway/github/review.go +++ b/internal/gateway/github/review.go @@ -175,7 +175,7 @@ func (c *Gateway) PullRequestLatestOpinionatedReviews(ctx context.Context, id ID } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { yield(nil, fmt.Errorf("list latest opinionated reviews (page %d): %w", pageNum, err)) return } @@ -483,7 +483,7 @@ func (c *Gateway) pullRequestReviewThreadListPage(ctx context.Context, id ID, fi } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, err } return &result.Node.ReviewThreads, nil @@ -519,7 +519,7 @@ func (c *Gateway) pullRequestReviewCommentsPage(ctx context.Context, id ID, firs } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, err } return &result.Node.Comments, nil diff --git a/internal/gateway/github/review_thread.go b/internal/gateway/github/review_thread.go index 4af2e13d..9a079bb8 100644 --- a/internal/gateway/github/review_thread.go +++ b/internal/gateway/github/review_thread.go @@ -88,7 +88,7 @@ func (c *Gateway) pullRequestReviewThreadsFirstPages(ctx context.Context, ids [] } } `) - if err := c.execute(ctx, query, struct { + if err := c.executeGQL(ctx, query, struct { IDs []ID `json:"ids"` }{ids}, &result); err != nil { return nil, fmt.Errorf("query pull request review threads: %w", err) @@ -126,7 +126,7 @@ func (c *Gateway) pullRequestReviewThreadsPage(ctx context.Context, id ID, first } } `) - if err := c.execute(ctx, query, variables, &result); err != nil { + if err := c.executeGQL(ctx, query, variables, &result); err != nil { return nil, fmt.Errorf("query pull request review threads page: %w", err) } return reviewThreads(&result.Node.ReviewThreads), nil diff --git a/internal/gateway/github/stack.go b/internal/gateway/github/stack.go new file mode 100644 index 00000000..1de1078e --- /dev/null +++ b/internal/gateway/github/stack.go @@ -0,0 +1,41 @@ +package github + +import ( + "context" + "fmt" +) + +// PullRequestStack describes a native pull request stack. +type PullRequestStack struct { + // Number is the number GitHub assigned to this stack. + // It identifies the stack within its repository and is unrelated to member + // pull request numbers or entry positions. + Number int + + // Members lists the open stack members from the base upward. + Members []PullRequestStackMember +} + +// PullRequestStackMember describes one open member of a native stack. +type PullRequestStackMember struct { + // Number is the repository-local pull request number. + Number int + + // Locked reports whether GitHub must preserve this member because it is in + // a merge queue or has auto-merge enabled. + Locked bool +} + +// CheckPullRequestStacks verifies that the repository exposes GitHub's native +// stack REST API without changing repository state. +// See https://docs.github.com/en/rest/pulls/stacks#list-pull-request-stacks. +func (c *Gateway) CheckPullRequestStacks( + ctx context.Context, + owner string, + repo string, +) error { + if err := c.getREST(ctx, []string{"repos", owner, repo, "stacks"}, nil); err != nil { + return fmt.Errorf("check pull request stacks: %w", err) + } + return nil +} diff --git a/internal/gateway/github/stack_lookup.go b/internal/gateway/github/stack_lookup.go new file mode 100644 index 00000000..5b4e81e6 --- /dev/null +++ b/internal/gateway/github/stack_lookup.go @@ -0,0 +1,358 @@ +package github + +import ( + "context" + "fmt" + "strconv" + "strings" +) + +// StackUpdatePullRequest is the compact pull request projection needed to +// reconcile native-stack membership. +type StackUpdatePullRequest struct { + // ID is the pull request's GraphQL node ID. + ID ID + + // State is the pull request lifecycle state. + State PullRequestState + + // HeadRefName and BaseRefName identify the current provider-facing branch + // relationship. + HeadRefName string + BaseRefName string + + // HeadRepositoryOwner is the login that owns the head repository. + HeadRepositoryOwner string + + // HeadRepositoryName is the head repository name. + HeadRepositoryName string + + // Stack is the native stack containing the pull request. + // It is nil when the pull request was not stacked in the initial query or + // when its referenced stack stopped resolving before the follow-up query. + Stack *PullRequestStack +} + +// PullRequestsForStackUpdate loads pull request eligibility and native-stack +// membership in input order. +// A nil result entry means that GitHub did not find the corresponding pull +// request. +// When Stack is non-nil, it includes all open members in base-up order. +// +// The result combines two API snapshots because GitHub exposes stack identity +// on the pull request and ordered membership on a separate stack node. +// If a referenced stack stops resolving between those requests, the pull +// request result remains present with Stack nil. +// +// GitHub exposes PullRequestStack as a GraphQL node, so this operation loads +// each unique stack once rather than repeating its entries for every member. +// See https://docs.github.com/en/graphql/reference/pulls#pullrequeststack. +func (c *Gateway) PullRequestsForStackUpdate( + ctx context.Context, + owner string, + repo string, + numbers []int, +) ([]*StackUpdatePullRequest, error) { + if len(numbers) == 0 { + return nil, nil + } + + variables := make(map[string]any, len(numbers)+2) + variables["owner"] = owner + variables["repo"] = repo + + // Build one aliased repository selection per pull request number: + // + // query($owner:String!$repo:String!$pr0:Int!$pr1:Int!) { + // repository(owner: $owner, name: $repo) { + // pr0: pullRequest(number: $pr0) { + // id state headRefName baseRefName + // headRepository { owner { login } name } + // stack { id } + // } + // pr1: pullRequest(number: $pr1) { + // state + // headRepository { owner { login } name } + // stack { id } + // } + // } + // } + var variableDefinitions strings.Builder + variableDefinitions.WriteString("$owner:String!,$repo:String!") + var selections strings.Builder + + // Indexed aliases preserve one response slot for every input number, + // including duplicate numbers and pull requests GitHub does not find. + for i, number := range numbers { + alias := "pr" + strconv.Itoa(i) + fmt.Fprintf(&variableDefinitions, ",$%s:Int!", alias) + if i > 0 { + selections.WriteByte(',') + } + fmt.Fprintf( + &selections, + "%[1]s:pullRequest(number: $%[1]s){id,state,headRefName,baseRefName,headRepository{owner{login},name},stack{id}}", + alias, + ) + variables[alias] = number + } + + var result struct { + Repository map[string]*struct { + ID ID `json:"id"` + State PullRequestState `json:"state"` + HeadRefName string `json:"headRefName"` + BaseRefName string `json:"baseRefName"` + HeadRepository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + Name string `json:"name"` + } `json:"headRepository"` + Stack *struct { + ID ID `json:"id"` + } `json:"stack"` + } `json:"repository"` + } + query := compactGraphQL(fmt.Sprintf(` + query(%s){ + repository(owner: $owner, name: $repo){%s} + } + `, variableDefinitions.String(), selections.String())) + if err := c.executeGQL(ctx, query, variables, &result); err != nil { + return nil, fmt.Errorf("query pull requests for stack update: %w", err) + } + + pullRequests := make([]*StackUpdatePullRequest, len(numbers)) + pullRequestsAwaitingStackByID := make(map[ID][]*StackUpdatePullRequest) + var stackIDsToResolve []ID + for i := range numbers { + res := result.Repository["pr"+strconv.Itoa(i)] + if res == nil { + continue + } + + pullRequest := &StackUpdatePullRequest{ + ID: res.ID, + State: res.State, + HeadRefName: res.HeadRefName, + BaseRefName: res.BaseRefName, + HeadRepositoryOwner: res.HeadRepository.Owner.Login, + HeadRepositoryName: res.HeadRepository.Name, + } + pullRequests[i] = pullRequest + + if res.Stack == nil { + continue + } + stackID := res.Stack.ID + if _, seen := pullRequestsAwaitingStackByID[stackID]; !seen { + stackIDsToResolve = append(stackIDsToResolve, stackID) + } + pullRequestsAwaitingStackByID[stackID] = append( + pullRequestsAwaitingStackByID[stackID], + pullRequest, + ) + } + if len(stackIDsToResolve) == 0 { + return pullRequests, nil + } + + // Resolve the ordered open members for every unique stack ID in one query. + // A node may disappear after the pull request query; in that case, leaving + // Stack nil preserves the best snapshot this non-atomic operation obtained. + resolvedStacksByID, err := c.pullRequestStacksByID(ctx, stackIDsToResolve) + if err != nil { + return nil, err + } + for stackID, awaitingPullRequests := range pullRequestsAwaitingStackByID { + resolvedStack, ok := resolvedStacksByID[stackID] + if !ok { + continue + } + for _, pullRequest := range awaitingPullRequests { + pullRequest.Stack = resolvedStack + } + } + return pullRequests, nil +} + +// pullRequestStacksByID loads every ordered open member of each native stack. +// The first page for all stacks is batched; stacks larger than one GraphQL page +// are completed with per-stack continuation queries. +func (c *Gateway) pullRequestStacksByID( + ctx context.Context, + stackIDs []ID, +) (map[ID]*PullRequestStack, error) { + var result struct { + Nodes []*struct { + ID ID `json:"id"` + Number int `json:"number"` + Entries pullRequestStackEntriesConnection `json:"entries"` + } `json:"nodes"` + } + // Build one batched query for the first page of every referenced stack: + // + // query($ids:[ID!]!) { + // nodes(ids: $ids) { + // ... on PullRequestStack { + // id number + // entries(first: 100) { + // nodes { pullRequest { + // number state + // mergeQueueEntry { id } + // autoMergeRequest { enabledAt } + // } } + // pageInfo { endCursor hasNextPage } + // } + // } + // } + // } + query := compactGraphQL(` + query($ids:[ID!]!){ + nodes(ids: $ids){ + ... on PullRequestStack{ + id,number, + entries(first: 100){ + nodes{pullRequest{ + number,state,mergeQueueEntry{id},autoMergeRequest{enabledAt} + }}, + pageInfo{endCursor,hasNextPage} + } + } + } + } + `) + if err := c.executeGQL(ctx, query, struct { + IDs []ID `json:"ids"` + }{stackIDs}, &result); err != nil { + return nil, fmt.Errorf("query pull request stacks: %w", err) + } + + stacksByID := make(map[ID]*PullRequestStack, len(result.Nodes)) + for _, res := range result.Nodes { + if res == nil { + continue + } + + stack := &PullRequestStack{Number: res.Number} + entries := &res.Entries + for pageNum := 1; ; pageNum++ { + for _, entry := range entries.Nodes { + pullRequest := entry.PullRequest + if pullRequest == nil || pullRequest.State != PullRequestStateOpen { + continue + } + stack.Members = append(stack.Members, PullRequestStackMember{ + Number: pullRequest.Number, + Locked: pullRequest.MergeQueueEntry != nil || + pullRequest.AutoMergeRequest != nil, + }) + } + if !entries.PageInfo.HasNextPage { + break + } + if entries.PageInfo.EndCursor == "" { + return nil, fmt.Errorf( + "query pull request stack %d entries: page %d has no end cursor", + res.Number, + pageNum, + ) + } + + nextEntries, err := c.pullRequestStackEntriesPage( + ctx, + res.ID, + entries.PageInfo.EndCursor, + ) + if err != nil { + return nil, fmt.Errorf( + "query pull request stack %d entries page %d: %w", + res.Number, + pageNum+1, + err, + ) + } + if nextEntries == nil { + stack = nil + break + } + entries = nextEntries + } + if stack != nil { + stacksByID[res.ID] = stack + } + } + + return stacksByID, nil +} + +// pullRequestStackEntriesPage loads one continuation page for a stack. +// It returns nil when the stack node no longer resolves. +func (c *Gateway) pullRequestStackEntriesPage( + ctx context.Context, + stackID ID, + after string, +) (*pullRequestStackEntriesConnection, error) { + var result struct { + Node *struct { + Entries pullRequestStackEntriesConnection `json:"entries"` + } `json:"node"` + } + // Build a continuation query for one stack's remaining entries: + // + // query($after:String!$id:ID!) { + // node(id: $id) { + // ... on PullRequestStack { + // entries(first: 100, after: $after) { + // nodes { pullRequest { + // number state + // mergeQueueEntry { id } + // autoMergeRequest { enabledAt } + // } } + // pageInfo { endCursor hasNextPage } + // } + // } + // } + // } + query := compactGraphQL(` + query($after:String!$id:ID!){ + node(id: $id){ + ... on PullRequestStack{ + entries(first: 100, after: $after){ + nodes{pullRequest{ + number,state,mergeQueueEntry{id},autoMergeRequest{enabledAt} + }}, + pageInfo{endCursor,hasNextPage} + } + } + } + } + `) + variables := struct { + After string `json:"after"` + ID ID `json:"id"` + }{after, stackID} + if err := c.executeGQL(ctx, query, variables, &result); err != nil { + return nil, err + } + if result.Node == nil { + return nil, nil + } + return &result.Node.Entries, nil +} + +type pullRequestStackEntriesConnection struct { + Nodes []struct { + PullRequest *struct { + Number int `json:"number"` + State PullRequestState `json:"state"` + MergeQueueEntry *struct{} `json:"mergeQueueEntry"` + AutoMergeRequest *struct{} `json:"autoMergeRequest"` + } `json:"pullRequest"` + } `json:"nodes"` + PageInfo struct { + EndCursor string `json:"endCursor"` + HasNextPage bool `json:"hasNextPage"` + } `json:"pageInfo"` +} diff --git a/internal/gateway/github/stack_mutation.go b/internal/gateway/github/stack_mutation.go new file mode 100644 index 00000000..9d2b1507 --- /dev/null +++ b/internal/gateway/github/stack_mutation.go @@ -0,0 +1,158 @@ +package github + +import ( + "context" + "fmt" + "strconv" +) + +const maxStackPullRequests = 100 + +// UnstackPullRequestStackInput identifies a native stack to dissolve. +type UnstackPullRequestStackInput struct { + // Owner is the login that owns the repository. + Owner string // required + + // Repo is the repository name. + Repo string // required + + // StackNumber identifies the stack within the repository. + StackNumber int // required +} + +// UnstackPullRequestStackResult reports members GitHub kept stacked. An empty +// result means that GitHub dissolved the complete stack. +type UnstackPullRequestStackResult struct { + RemainingPullRequests []int +} + +// UnstackPullRequestStack dissolves a native pull request stack. GitHub may +// preserve queued or auto-merge members and return them in a smaller stack. +// See https://docs.github.com/en/rest/pulls/stacks#unstack-a-pull-request-stack. +func (c *Gateway) UnstackPullRequestStack( + ctx context.Context, + input *UnstackPullRequestStackInput, +) (*UnstackPullRequestStackResult, error) { + var res struct { + PullRequests []struct { + Number int `json:"number"` + } `json:"pull_requests"` + } + if err := c.deleteREST( + ctx, + []string{ + "repos", input.Owner, input.Repo, "stacks", + strconv.Itoa(input.StackNumber), + }, + nil, + &res, + ); err != nil { + return nil, fmt.Errorf("unstack pull request stack: %w", err) + } + + result := &UnstackPullRequestStackResult{ + RemainingPullRequests: make([]int, len(res.PullRequests)), + } + for i, pullRequest := range res.PullRequests { + result.RemainingPullRequests[i] = pullRequest.Number + } + return result, nil +} + +// CreatePullRequestStackInput identifies the pull requests for a new stack. +type CreatePullRequestStackInput struct { + // Owner is the login that owns the repository. + Owner string // required + + // Repo is the repository name. + Repo string // required + + // PullRequests lists pull request numbers from the base upward. + // GitHub accepts between 2 and 100 members. + PullRequests []int // required +} + +// CreatePullRequestStack creates a stack from pull requests ordered from the +// base upward. +// See https://docs.github.com/en/rest/pulls/stacks#create-a-pull-request-stack. +func (c *Gateway) CreatePullRequestStack( + ctx context.Context, + input *CreatePullRequestStackInput, +) error { + if err := validateStackPullRequestCount(len(input.PullRequests), 2); err != nil { + return err + } + + req := struct { + PullRequests []int `json:"pull_requests"` + }{PullRequests: input.PullRequests} + if err := c.postREST( + ctx, + []string{"repos", input.Owner, input.Repo, "stacks"}, + &req, + nil, + ); err != nil { + return fmt.Errorf("create pull request stack: %w", err) + } + return nil +} + +// AddPullRequestsToStackInput identifies pull requests to add to an existing +// stack. +type AddPullRequestsToStackInput struct { + // Owner is the login that owns the repository. + Owner string // required + + // Repo is the repository name. + Repo string // required + + // StackNumber identifies the stack within the repository. + StackNumber int // required + + // PullRequests lists pull request numbers from the current top upward. + // GitHub accepts between 1 and 100 members. + PullRequests []int // required +} + +// AddPullRequestsToStack adds pull requests above an existing stack. +// See https://docs.github.com/en/rest/pulls/stacks#add-pull-requests-to-a-pull-request-stack. +func (c *Gateway) AddPullRequestsToStack( + ctx context.Context, + input *AddPullRequestsToStackInput, +) error { + if err := validateStackPullRequestCount(len(input.PullRequests), 1); err != nil { + return err + } + + req := struct { + PullRequests []int `json:"pull_requests"` + }{PullRequests: input.PullRequests} + if err := c.postREST( + ctx, + []string{ + "repos", + input.Owner, + input.Repo, + "stacks", + strconv.Itoa(input.StackNumber), + "add", + }, + &req, + nil, + ); err != nil { + return fmt.Errorf("add pull requests to stack: %w", err) + } + return nil +} + +func validateStackPullRequestCount(count, minimum int) error { + if count < minimum || count > maxStackPullRequests { + return fmt.Errorf( + "pull request count must be between %d and %d: %d", + minimum, + maxStackPullRequests, + count, + ) + } + return nil +} diff --git a/internal/gateway/github/stack_test.go b/internal/gateway/github/stack_test.go new file mode 100644 index 00000000..33d48364 --- /dev/null +++ b/internal/gateway/github/stack_test.go @@ -0,0 +1,162 @@ +package github + +import ( + "encoding/json" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGateway_CheckPullRequestStacks(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/repos/octo/hello/stacks", r.URL.Path) + return restJSONResponse(http.StatusOK, `{"stacks":[]}`), nil + })) + + require.NoError(t, gateway.CheckPullRequestStacks(t.Context(), "octo", "hello")) +} + +func TestGateway_CreatePullRequestStack(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "api.github.com", r.URL.Host) + assert.Equal(t, "/repos/octo/hello/stacks", r.URL.Path) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + assert.Equal(t, restMediaType, r.Header.Get("Accept")) + assert.Equal(t, restAPIVersion, r.Header.Get("X-GitHub-Api-Version")) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + assert.JSONEq(t, `{"pull_requests":[101,102]}`, string(body)) + return restJSONResponse(http.StatusCreated, `{}`), nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: []int{101, 102}, + }) + require.NoError(t, err) +} + +func TestGateway_CreatePullRequestStackCount(t *testing.T) { + for _, tt := range []struct { + name string + count int + }{ + {name: "TooFew", count: 1}, + {name: "TooMany", count: 101}, + } { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("HTTP request made with invalid pull request count") + return nil, nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: make([]int, tt.count), + }) + assert.ErrorContains(t, err, "between 2 and 100") + }) + } +} + +func TestGateway_CreatePullRequestStackMaximum(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + var body struct { + PullRequests []int `json:"pull_requests"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Len(t, body.PullRequests, 100) + return restJSONResponse(http.StatusCreated, `{}`), nil + })) + + err := gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{ + Owner: "octo", + Repo: "hello", + PullRequests: make([]int, 100), + }) + require.NoError(t, err) +} + +func TestGateway_AddPullRequestsToStack(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/repos/octo/hello/stacks/42/add", r.URL.Path) + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + assert.JSONEq(t, `{"pull_requests":[103]}`, string(body)) + return restJSONResponse(http.StatusOK, `{}`), nil + })) + + err := gateway.AddPullRequestsToStack(t.Context(), &AddPullRequestsToStackInput{ + Owner: "octo", + Repo: "hello", + StackNumber: 42, + PullRequests: []int{103}, + }) + require.NoError(t, err) +} + +func TestGateway_AddPullRequestsToStackCount(t *testing.T) { + for _, tt := range []struct { + name string + count int + }{ + {name: "TooFew", count: 0}, + {name: "TooMany", count: 101}, + } { + t.Run(tt.name, func(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("HTTP request made with invalid pull request count") + return nil, nil + })) + + err := gateway.AddPullRequestsToStack(t.Context(), &AddPullRequestsToStackInput{ + Owner: "octo", + Repo: "hello", + StackNumber: 42, + PullRequests: make([]int, tt.count), + }) + assert.ErrorContains(t, err, "between 1 and 100") + }) + } +} + +func TestGateway_UnstackPullRequestStack(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodDelete, r.Method) + assert.Equal(t, "/repos/octo/hello/stacks/42", r.URL.Path) + return restJSONResponse(http.StatusOK, `{ + "pull_requests":[{"number":102},{"number":103}] + }`), nil + })) + + got, err := gateway.UnstackPullRequestStack(t.Context(), &UnstackPullRequestStackInput{ + Owner: "octo", + Repo: "hello", + StackNumber: 42, + }) + require.NoError(t, err) + assert.Equal(t, []int{102, 103}, got.RemainingPullRequests) +} + +func TestGateway_UnstackPullRequestStackComplete(t *testing.T) { + gateway := newTestGateway(t, roundTripFunc(func(*http.Request) (*http.Response, error) { + return restJSONResponse(http.StatusNoContent, ``), nil + })) + + got, err := gateway.UnstackPullRequestStack(t.Context(), &UnstackPullRequestStackInput{ + Owner: "octo", + Repo: "hello", + StackNumber: 42, + }) + require.NoError(t, err) + assert.Empty(t, got.RemainingPullRequests) +} diff --git a/internal/gateway/github/status_check.go b/internal/gateway/github/status_check.go index 1af95970..9780bebb 100644 --- a/internal/gateway/github/status_check.go +++ b/internal/gateway/github/status_check.go @@ -149,7 +149,7 @@ func (c *Gateway) statusChecksPage(ctx context.Context, id ID, first int, after ID ID `json:"id"` }{after, id} var result statusChecksResult - if err := c.execute(ctx, query, vars, &result); err != nil { + if err := c.executeGQL(ctx, query, vars, &result); err != nil { return nil, fmt.Errorf("query status checks: %w", err) } diff --git a/internal/gateway/github/template.go b/internal/gateway/github/template.go index 797ece53..3b3dc1e9 100644 --- a/internal/gateway/github/template.go +++ b/internal/gateway/github/template.go @@ -32,7 +32,7 @@ func (c *Gateway) ChangeTemplates(ctx context.Context, owner, repo string) ([]*C } } `) - if err := c.execute(ctx, query, vars, &result); err != nil { + if err := c.executeGQL(ctx, query, vars, &result); err != nil { return nil, fmt.Errorf("query templates: %w", err) } return result.Repository.PullRequestTemplates, nil