Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,10 @@ gen-api-live:
## source figures its content does not give, has a fine-grained half (route
## authorization, the permission vocabulary, the public set, GraphQL
## authorization) that is missing, too small or holds a shape a reader would
## misread, or is past the shared staleness window (cmd/internal/provenance).
## misread, has a keyset half (where each route serves a keyset page, by which
## parameter, and the probe of GitLab's paginators) that is missing, too small,
## unreadable or silent on a route declaring a cursor, or is past the shared
## staleness window (cmd/internal/provenance).
## No Docker and no network, so it is a gate.
check-api-live:
go run ./cmd/gen_api_live/ -check
Expand Down
19 changes: 19 additions & 0 deletions cmd/audit_1to1/internal/paths/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,17 @@ type Summary struct {
// PaginationGrain says what the collection counts were asked at, for the
// same reason Grain does above: the number is what gets quoted.
PaginationGrain string `json:"pagination_endpoint_grain"`
// The keyset counts are R-PAGE's other half: how many actions offer
// page_token, how many of them on a route this server can page by
// keyset, and the findings by problem, with the ones no declaration
// accounts for. See [KeysetPagination].
PageTokenActions int `json:"page_token_actions"`
PageTokenOnKeysetRoutes int `json:"page_token_actions_on_keyset_routes"`
PageTokenOnOffsetRoutes int `json:"page_token_on_offset_routes"`
KeysetRoutesNotOffered int `json:"keyset_routes_without_page_token"`
KeysetNoNextPageToken int `json:"keyset_routes_without_next_page_token"`
KeysetNeverFollowed int `json:"keyset_routes_never_followed"`
KeysetFindingsUndeclared int `json:"keyset_findings_undeclared"`
// SDKGraphQLDocuments and SDKGraphQLRefused count the documents client-go
// builds inside its own module and the ones the pinned schema will not
// accept. They are beside GraphQLDocuments and GraphQLRefused rather than
Expand Down Expand Up @@ -419,6 +430,13 @@ func buildReport(ctx context.Context, root string, opts Options) (Report, error)
CollectionsUnpaginated: pagination.Collections.Unpaginated,
CollectionsUndeclared: pagination.Collections.Undeclared,
PaginationGrain: paginationGrain,
PageTokenActions: pagination.Keyset.Offering,
PageTokenOnKeysetRoutes: pagination.Keyset.OnKeyset,
PageTokenOnOffsetRoutes: pagination.Keyset.count(keysetOnOffsetRoute),
KeysetRoutesNotOffered: pagination.Keyset.count(keysetNotOffered),
KeysetNoNextPageToken: pagination.Keyset.count(keysetNoNextPageToken),
KeysetNeverFollowed: pagination.Keyset.count(keysetNeverFollowed),
KeysetFindingsUndeclared: pagination.Keyset.undeclared(),
SDKGraphQLDocuments: sdkGraphQL.Documents,
SDKGraphQLRefused: len(sdkGraphQL.Refusals),
E2EActionsObserved: len(e2e.Issuing),
Expand All @@ -434,6 +452,7 @@ func buildReport(ctx context.Context, root string, opts Options) (Report, error)
report.SilentOwners = keepUndeclared(report.SilentOwners)
report.Endpoints.Undocumented = keepUndeclaredEndpoints(report.Endpoints.Undocumented)
report.Pagination.Unpaginated = keepUndeclaredCollections(report.Pagination.Unpaginated)
report.Pagination.Keyset.Findings = keepUndeclaredKeyset(report.Pagination.Keyset.Findings)
// The SDK document listing and the per-action observation keep only
// their findings too: the document set and the observed actions are
// context, and -gaps-only asks for the work.
Expand Down
84 changes: 84 additions & 0 deletions cmd/audit_1to1/internal/paths/analyze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ import (
"go/token"
"os"
"path/filepath"
"reflect"
"slices"
"strings"
"testing"

"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/actionrequests"
"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/apidocs"
"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/graphqldocs"
"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/requestinventory"
Expand Down Expand Up @@ -272,6 +275,61 @@ func TestRun_GapsOnly_KeepsTheWorkAndDropsTheContext(t *testing.T) {
})
}

// TestRun_GapsOnly_KeepsTheKeysetWorkAndDropsItsDeclaredFindings verifies the
// flag on the keyset half of R-PAGE: a list a route declaration answers is
// context, so -gaps-only leaves it out while the full report keeps it, and the
// summary counts the half's findings by problem either way.
func TestRun_GapsOnly_KeepsTheKeysetWorkAndDropsItsDeclaredFindings(t *testing.T) {
withPaginationDeclarations(t, nil)
withKeysetDeclarations(t, []keysetRouteDeclaration{{
Route: "GET /projects/:id/anonymous", Category: categoryKeysetNotFollowable, Reason: "only a signed-out caller reaches it",
}})
withDeclarations(t, map[string]silentOwnerDeclaration{})
root := recordIn(t, map[string]response{
"GET /projects/:id/anonymous": cursorRoute,
"GET /projects/:id/things": {Params: []string{"id", "page", "per_page"}},
})
makeToolsPackage(t, root, "things")
writeActionRequests(t, root, []actionrequests.RecordAction{
sending("thing.list", "things.List", "GET /projects/:id/things"),
sending("thing.search", "things.Search", "GET /projects/:id/anonymous"),
})
stubInputs(t, []requestinventory.Row{
{Package: "internal/tools/things", Kind: requestinventory.KindREST, Method: "GET", Path: "/projects/:project_id/things"},
}, []requestinventory.Action{
listAction("thing.list", "things", reflect.TypeFor[fixturePaginatedListOutput](), withPageToken),
listAction("thing.search", "things", reflect.TypeFor[fixturePaginatedListOutput](), withoutPageToken),
}, graphqldocs.Result{})

full, _, err := Run(t.Context(), root, Options{})
if err != nil {
t.Fatalf("Run() error = %v", err)
}
gaps, _, err := Run(t.Context(), root, Options{GapsOnly: true})
if err != nil {
t.Fatalf("Run() error = %v", err)
}

if got := decode(t, full).Pagination.Keyset.Findings; len(got) != 2 {
t.Errorf("the full report lists %+v, want the offset finding and the declared list", got)
}
kept := decode(t, gaps).Pagination.Keyset.Findings
if len(kept) != 1 || kept[0].Action != "thing.list" || kept[0].Problem != keysetOnOffsetRoute {
t.Errorf("the gaps-only report lists %+v, want the one finding no declaration answers", kept)
}
t.Run("the summary counts the half by problem", func(t *testing.T) {
summary := decode(t, gaps).Summary
got := []int{
summary.PageTokenActions, summary.PageTokenOnKeysetRoutes, summary.PageTokenOnOffsetRoutes,
summary.KeysetRoutesNotOffered, summary.KeysetNoNextPageToken, summary.KeysetNeverFollowed,
summary.KeysetFindingsUndeclared,
}
if want := []int{1, 0, 1, 1, 0, 0, 1}; !slices.Equal(got, want) {
t.Errorf("keyset summary = %v, want %v", got, want)
}
})
}

// TestRun_GapsOnly_DropsTheListingsOfClientGosDocuments verifies the same flag
// on the SDK section: the shells client-go writes, whether the collector
// rendered them or left them shells, are a listing of what was read rather
Expand Down Expand Up @@ -634,4 +692,30 @@ func TestRun_TheRealTree_PassesItsOwnGate(t *testing.T) {
t.Errorf("only %d of %d catalog actions are owned by a package that recorded a request or declared why it did not; the recording has regressed",
accounted, report.Summary.CatalogActions)
}
// The keyset half has to have run for its route declarations to have been
// held to the tree at all, and the summary quotes its findings: each
// figure is the count of the findings it names.
keyset := report.Pagination.Keyset
if !keyset.Ran || keyset.Offering == 0 || keyset.OnKeyset == 0 {
t.Errorf("keyset half = ran %t, %d offering page_token, %d on keyset routes; want it run over the catalog",
keyset.Ran, keyset.Offering, keyset.OnKeyset)
}
problems := map[string]int{}
for _, finding := range keyset.Findings {
problems[finding.Problem]++
}
summary := report.Summary
got := []int{
summary.PageTokenActions, summary.PageTokenOnKeysetRoutes, summary.PageTokenOnOffsetRoutes,
summary.KeysetRoutesNotOffered, summary.KeysetNoNextPageToken, summary.KeysetNeverFollowed,
summary.KeysetFindingsUndeclared,
}
want := []int{
keyset.Offering, keyset.OnKeyset, problems[keysetOnOffsetRoute],
problems[keysetNotOffered], problems[keysetNoNextPageToken], problems[keysetNeverFollowed],
keyset.undeclared(),
}
if !slices.Equal(got, want) {
t.Errorf("keyset summary = %v, want the counts of the findings it names, %v", got, want)
}
}
25 changes: 20 additions & 5 deletions cmd/audit_1to1/internal/paths/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,13 @@
// which returns a bare array of tokens while GitLab serves twenty at a time. A
// caller cannot tell it has one page and cannot ask for the next.
//
// The oracle is the live record's params: 318 of its 2152 mounted routes declare
// per_page at 19.4.1-ee, 311 of those declare page beside it and the other seven
// take a cursor or a page_token. That is a far stronger statement than a guess
// from an endpoint's name, and it is available because the record asks the
// router rather than reading prose.
// The oracle is the live record. For offset pagination it is the params: 312 of
// its 2192 mounted routes declare page beside per_page at 19.5.0-ee. That is a
// far stronger statement than a guess from an endpoint's name, and it is
// available because the record asks the router rather than reading prose. For
// keyset pagination the params cannot say, since GitLab reads pagination=keyset
// without declaring it, and the oracle is the keyset field of the record's
// schema 5, the 53 routes whose code reaches a keyset paginator.
//
// An action is judged when its output is a collection envelope, which is exactly
// one content field that is a list of objects, with this server's own framing
Expand Down Expand Up @@ -339,6 +341,19 @@
// first and after and publishing the connection's count and the forward half
// of pageInfo, and upstream-bugs row 104 records the client-go half.
//
// A keyset page says where it ends in a next link, so the rule asks a list
// GitLab can page by keyset other questions (issue 1165), at the action's own
// grain, since the request record the handlers were derived into names the
// routes each action sends: whether page_token is offered where a route can be
// paged by keyset and only there, whether a list on such a route offers it,
// and whether an action that offers it publishes the next_page_token its next
// page is asked with and was recorded sending a route its cursor. A route the
// record marks and GitLab serves this server no followable keyset page on, a
// helper whose keyset branch only a signed-out caller reaches or a model
// paginate_with_strategies answers 405 for, is declared in
// pagination_declarations.go with what GitLab's handler does. See
// [KeysetPagination].
//
// # Is a param sent that the caller never asked to send
//
// The first of the two checks that read values rather than names, and the
Expand Down
31 changes: 24 additions & 7 deletions cmd/audit_1to1/internal/paths/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@ import (
// here. What would remove them is an inventory that named the action, which is a
// different piece of work: nothing the recorder sees today can supply it.
//
// # Keyset pagination
//
// Everything above is about a list that says nothing of where it ends. A list
// GitLab can page by keyset says where it ends in a next link, and is asked
// different questions, at the action's own grain rather than the package's:
// whether page_token is offered where GitLab can page by keyset and only
// there, and whether the next_page_token it is followed by is published and
// was ever sent back. Those are [KeysetPagination], the Keyset field below,
// read off the keyset field of the live record's schema 5 and the request
// record the handlers were derived into.
//
// # It reports and does not gate
//
// A finding is a surface change, and the surface is not this command's to
Expand Down Expand Up @@ -119,6 +130,10 @@ type PaginationCheck struct {
// Unpaginated is the finding list: an action reading a collection, in a
// package recorded calling a paginated endpoint, publishing no pagination.
Unpaginated []UnpaginatedCollection `json:"unpaginated,omitempty"`
// Keyset is the other half of the question: whether page_token is offered
// where GitLab can page by keyset, and only there, and whether a keyset
// page can be followed. See [KeysetPagination].
Keyset KeysetPagination `json:"keyset"`
}

// RoutePagination is what the live record says about pagination, before any of
Expand All @@ -129,10 +144,10 @@ type RoutePagination struct {
// Offset declares page and per_page: the answer's headers carry the page,
// the total and the pages either side of it.
Offset int `json:"offset_paginated"`
// Keyset declares per_page without page, so the next page is asked for with
// the cursor or page_token the Link header carries. A page-and-total block
// is the wrong shape to publish for one of these, which is why they are
// counted apart rather than folded in.
// Keyset is every route whose code reaches a keyset paginator, as the
// record's keyset field says, so the next page is asked for with the
// cursor or page_token the Link header carries. Most of them page by
// offset too, which is GitLab's default, so a route can count in both.
Keyset int `json:"keyset_paginated"`
}

Expand Down Expand Up @@ -256,10 +271,11 @@ func paginationCheck(root string, requests []requestinventory.Row, actions []req
check := PaginationCheck{Ran: true, Record: apilive.FileName, Grain: paginationGrain}
for _, route := range record.Routes {
check.Routes.Routes++
switch offset, keyset := routePagination(route); {
case offset:
offset, keyset := routePagination(route)
if offset {
check.Routes.Offset++
case keyset:
}
if keyset {
check.Routes.Keyset++
}
}
Expand All @@ -276,6 +292,7 @@ func paginationCheck(root string, requests []requestinventory.Row, actions []req
check.Recorded.KeysetPackages = len(keysetByPackage)

check.Collections, check.Unpaginated = unpaginatedCollections(actions, offsetByPackage, keysetByPackage)
check.Keyset = keysetPagination(root, record, requests, actions)
return check
}

Expand Down
Loading
Loading