Offer page_token only where GitLab pages by keyset, and gate R-PAGE on it

cmd/audit_1to1/internal/paths/analyze.go

@@ -255,16 +255,24 @@ const observedGrain = "package: an action counts as observed when the package th
 // found something undeclared: it is a candidate list, for the reasons in
 // [EndpointCheck].
 //
-// So are the two shape comparisons and the pagination check, each for its own
-// recorded reason. What their declaration tables do reach the gate through is
-// StaleDeclarations: a finding is a candidate and a claim that has stopped being
-// true is not.
+// So are the two shape comparisons and the collection half of the pagination
+// check, each for its own recorded reason. What their declaration tables do
+// reach the gate through is StaleDeclarations: a finding is a candidate and a
+// claim that has stopped being true is not.
+//
+// The keyset half of the pagination check does gate, on the findings no route
+// declaration answers (issue 1165): with the keyset input taken off every
+// action whose route GitLab pages by offset alone, each such finding is a
+// list offering a way to page that GitLab ignores or refuses, or a keyset
+// list a caller cannot page the cheaper way, and a re-recorded live record
+// that moves a route between the two is held to the same rule.
 func (s Summary) clean() bool {
 	return s.GraphQLRefused == 0 &&
 		s.UndeclaredSilent == 0 &&
 		s.StaleDeclarations == 0 &&
 		s.ActionsUnmapped == 0 &&
-		s.UndeclaredEndpoints == 0
+		s.UndeclaredEndpoints == 0 &&
+		s.KeysetFindingsUndeclared == 0
 }
 
 // GraphQLRefusal is one document the schema will not accept.

cmd/audit_1to1/internal/paths/pagination_declarations.go

@@ -235,11 +235,6 @@ var declaredKeysetRoutes = []keysetRouteDeclaration{
 		Category: categoryKeysetNotFollowable,
 		Reason:   "the handler pages PersonalAccessTokensFinder's relation with paginate_with_strategies (lib/api/project_service_accounts.rb:217), and PersonalAccessToken declares no supported_keyset_orderings, so keyset_paginator takes Keyset::Pager, whose availability check admits Project alone, and answers every keyset request with 405 (lib/api/helpers/pagination_strategies.rb:30-43). The record's cursor is CursorPager's, which GitLab never reaches here.",
 	},
-	{
-		Route:    "GET /users/:user_id/contributed_projects",
-		Category: categoryKeysetNotFollowable,
-		Reason:   "present_projects reaches Keyset::Pager as on the other project lists (lib/api/projects.rb:248), but the handler builds ContributedProjectsFinder with no params (lib/api/projects.rb:396) and Keyset::Pager#paginate only limits the relation (lib/gitlab/pagination/keyset/pager.rb:13), so the id_after or id_before of the next link GitLab writes is never applied and every keyset page is the first again (upstream-bugs row 112).",
-	},
 }
 
 // declaredKeysetRoute finds the declaration of one route.

cmd/audit_1to1/internal/paths/pagination_keyset.go

@@ -60,14 +60,16 @@ import (
 // there. An instance that leaves the flag off ignores the parameter, which
 // costs a caller nothing.
 //
-// # It reports and does not gate
+// # It gates on what no declaration answers
 //
-// For the reason the collection half gives: every finding is a change to the
-// published surface. Most of what it reports against the 19.5.0 record is
-// page_token offered on a route GitLab pages by offset alone, which an action
-// publishes whenever its input embeds [toolutil.KeysetPaginationInput],
-// whatever its route does, and taking it out of those inputs is a change of
-// the surface 182 actions publish.
+// Against the 19.5.0 record it once reported 182 actions offering page_token
+// on a route GitLab pages by offset alone, which an action published whenever
+// its input embedded [toolutil.KeysetPaginationInput], whatever its route did.
+// With the keyset input taken off those inputs (issue 1165), every undeclared
+// finding is a change the surface owes, so [Summary.clean] counts them: an
+// action that offers page_token where no route pages by keyset, or a keyset
+// list that does not offer it, fails the run until its input changes or a
+// route declaration says why it cannot.
 type KeysetPagination struct {
 	// Ran is false when the request record could not be read, which is the
 	// one input this half reads that the collection half does not.

cmd/audit_1to1/internal/paths/pagination_keyset_test.go

@@ -9,8 +9,10 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/jmrplens/gitlab-mcp-server/v3/cmd/audit_1to1/internal/structs"
 	"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/actionrequests"
 	"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/apilive"
+	"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/graphqldocs"
 	"github.com/jmrplens/gitlab-mcp-server/v3/cmd/internal/requestinventory"
 	"github.com/jmrplens/gitlab-mcp-server/v3/internal/toolutil"
 )
@@ -476,3 +478,69 @@ func TestPaginationCheck_KeysetFindings_AreOrderedByPackageActionAndProblem(t *t
 		t.Errorf("findings = %v, want %v", got, want)
 	}
 }
+
+// TestRun_AnUndeclaredKeysetFinding_FailsTheGate verifies that the keyset
+// half gates on what no declaration answers (issue 1165). An action offering
+// page_token on a route GitLab pages by offset alone fails the run, and so
+// does a list that sends a keyset route without offering it; the same list on
+// a route a declaration says this server cannot page by keyset passes, as
+// does an offset list that offers page and per_page alone.
+func TestRun_AnUndeclaredKeysetFinding_FailsTheGate(t *testing.T) {
+	cases := []struct {
+		name      string
+		input     map[string]any
+		route     string
+		declared  bool
+		wantClean bool
+	}{
+		{name: "page_token on an offset route", input: withPageToken, route: "GET /projects/:id/things"},
+		{name: "a keyset route without page_token", input: withoutPageToken, route: "GET /projects/:id/cursors"},
+		{
+			name: "a keyset route a declaration says cannot be followed", input: withoutPageToken,
+			route: "GET /projects/:id/cursors", declared: true, wantClean: true,
+		},
+		{name: "an offset list offering page and per_page", input: withoutPageToken, route: "GET /projects/:id/things", wantClean: true},
+	}
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			withPaginationDeclarations(t, nil)
+			var declarations []keysetRouteDeclaration
+			if testCase.declared {
+				declarations = []keysetRouteDeclaration{{
+					Route: testCase.route, Category: categoryKeysetNotFollowable, Reason: "only a signed-out caller reaches it",
+				}}
+			}
+			withKeysetDeclarations(t, declarations)
+			withDeclarations(t, map[string]silentOwnerDeclaration{})
+			// The shape tables are about the real tree, and against this
+			// one every entry would be stale and fail the run on its own.
+			stubTypeGrainInputs(t, structs.Pairings{}, nil, nil)
+			root := recordIn(t, map[string]response{
+				"GET /projects/:id/cursors": 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", testCase.route)})
+			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](), testCase.input),
+			}, graphqldocs.Result{})
+
+			content, clean, err := Run(t.Context(), root, Options{})
+			if err != nil {
+				t.Fatalf("Run() error = %v", err)
+			}
+			if clean != testCase.wantClean {
+				t.Errorf("Run() clean = %t, want %t:\n%s", clean, testCase.wantClean, content)
+			}
+			wantUndeclared := 1
+			if testCase.wantClean {
+				wantUndeclared = 0
+			}
+			if got := decode(t, content).Summary.KeysetFindingsUndeclared; got != wantUndeclared {
+				t.Errorf("keyset_findings_undeclared = %d, want %d", got, wantUndeclared)
+			}
+		})
+	}
+}