From dcd3bef32cfd58de8183af75a4bd52909162ef84 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sun, 23 Aug 2026 07:21:05 -0700 Subject: [PATCH] github: Support native stack operations GitHub exposes native stack and asynchronous merge operations through REST alongside the existing GraphQL reads. Keep both transports behind Gateway so forge callers use repository-domain operations and the wire representations remain private. Expose only the stack mutations and asynchronous merge state needed by the forge layer. Attach to an existing pending merge without changing its requested behavior. Stack reconciliation needs current bases, head repository identity, locked member state, and native stack membership. Load that projection in input order with batched GraphQL queries, resolve each distinct stack once, detect API availability, and provide explicit stack dissolution for structural updates. --- internal/gateway/github/async_merge.go | 193 ++++++++++ internal/gateway/github/async_merge_test.go | 243 ++++++++++++ internal/gateway/github/comment.go | 2 +- internal/gateway/github/error.go | 8 +- internal/gateway/github/gateway.go | 166 +++----- internal/gateway/github/gateway_test.go | 24 +- internal/gateway/github/graphql.go | 108 +++++- internal/gateway/github/identity.go | 2 +- internal/gateway/github/label.go | 2 +- internal/gateway/github/pull_request_find.go | 6 +- .../gateway/github/pull_request_find_test.go | 30 +- internal/gateway/github/pull_request_id.go | 2 +- .../github/pull_request_merge_range.go | 194 ++++++++++ .../github/pull_request_merge_range_test.go | 164 ++++++++ .../github/pull_request_mergeability.go | 2 +- .../gateway/github/pull_request_metadata.go | 2 +- .../github/pull_request_stack_lookup_test.go | 261 +++++++++++++ .../gateway/github/pull_request_status.go | 2 +- internal/gateway/github/ref.go | 2 +- internal/gateway/github/repository.go | 2 +- internal/gateway/github/rest.go | 173 +++++++++ internal/gateway/github/rest_test.go | 156 ++++++++ internal/gateway/github/review.go | 6 +- internal/gateway/github/review_thread.go | 4 +- internal/gateway/github/stack.go | 41 ++ internal/gateway/github/stack_lookup.go | 358 ++++++++++++++++++ internal/gateway/github/stack_mutation.go | 158 ++++++++ internal/gateway/github/stack_test.go | 162 ++++++++ internal/gateway/github/status_check.go | 2 +- internal/gateway/github/template.go | 2 +- 30 files changed, 2326 insertions(+), 151 deletions(-) create mode 100644 internal/gateway/github/async_merge.go create mode 100644 internal/gateway/github/async_merge_test.go create mode 100644 internal/gateway/github/pull_request_merge_range.go create mode 100644 internal/gateway/github/pull_request_merge_range_test.go create mode 100644 internal/gateway/github/pull_request_stack_lookup_test.go create mode 100644 internal/gateway/github/rest.go create mode 100644 internal/gateway/github/rest_test.go create mode 100644 internal/gateway/github/stack.go create mode 100644 internal/gateway/github/stack_lookup.go create mode 100644 internal/gateway/github/stack_mutation.go create mode 100644 internal/gateway/github/stack_test.go diff --git a/internal/gateway/github/async_merge.go b/internal/gateway/github/async_merge.go new file mode 100644 index 000000000..f3a8c14c0 --- /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 000000000..24a55f96c --- /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 0f941124a..e29bd055b 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 887e3aed1..684093959 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 d32e76a9f..239cbc73d 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 8286e9474..4fd836c26 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 3a2b9f344..620850738 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 1899b0a5a..e15e647fa 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 a733ead94..da94a1777 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 9ecaa51b2..5b2385ca6 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 43c0ebb27..ac14f01d3 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 ad0fdf69d..72d998ef5 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 000000000..bd6e65d42 --- /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 000000000..888df94d4 --- /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 9458cf708..d11c63726 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 29335cc1f..aaa015c1c 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 000000000..9c1daeaf2 --- /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 169193bb7..ec290436d 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 a6cf280dd..0aab62633 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 61a117d32..0d4082c9a 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 000000000..be32855cd --- /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 000000000..abd0cadd8 --- /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 d12b14d76..326665a31 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 4af2e13d2..9a079bb81 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 000000000..1de1078ea --- /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 000000000..5b4e81e6f --- /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 000000000..9d2b1507c --- /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 000000000..33d483643 --- /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 1af959709..9780bebb3 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 797ece539..3b3dc1e92 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