Repository navigation
Record which routes can page by keyset at schema 5, and hold page_token to them in R-PAGE - #1574
Conversation
|
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Schema and keyset discovery cmd/internal/apilive/*, cmd/gen_api_live/introspect.rb, docs/development/gitlab-api-live.json, docs/development/request-paths.md, cmd/gen_action_grants/internal/join/doc.go |
Schema 5 records paginator probes, route sites, cursor parameters, and keyset-route counts. The introspector scans route code and emits the data; the committed snapshot and schema references are updated. |
Validate and generate the live record cmd/gen_api_live/*, Makefile |
The generator validates paginator probes, route sites, cursor declarations, and the minimum keyset-route count. Reports include keyset counts, and generation refuses records with keyset validation problems. |
Classify offset and keyset routes cmd/audit_1to1/internal/paths/pagination.go, cmd/audit_1to1/internal/paths/shapes.go, cmd/audit_1to1/internal/paths/pagination_test.go, cmd/audit_1to1/internal/paths/shapes_test.go, cmd/audit_1to1/internal/paths/doc.go |
Offset pagination requires both page and per_page. Keyset pagination comes from recorded paginator sites, and a route can be classified as both. |
Audit action tokens and follow-up requests cmd/audit_1to1/internal/paths/pagination_keyset.go, cmd/audit_1to1/internal/paths/pagination_declarations.go, cmd/audit_1to1/internal/paths/analyze.go, cmd/audit_1to1/internal/paths/*_test.go |
The audit checks action token inputs, pagination outputs, and recorded cursor requests. It reports findings, tracks non-followable route declarations, and adds keyset summary counts and gaps-only filtering. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant LiveRecord as API-live record
participant Audit as R-PAGE audit
participant RequestInventory
participant Report
Audit->>LiveRecord: Read route keyset sites and cursor parameters
Audit->>RequestInventory: Read action and request data
Audit->>Report: Record token, output, and cursor-use findings
Possibly related issues
- Re-pin the REST live record, the fine-grained table and the GraphQL schema on GitLab 19.6.0 when it ships #1492 — This change updates the live record to schema 5 with keyset data; the issue describes a subsequent live-record re-pin as a follow-up requirement.
- mr_review.changes_get accepts order_by, sort, pagination and page_token, which its route ignores #1530 — The new R-PAGE checks evaluate action pagination inputs against route pagination behavior, including the type of mismatch described in this issue.
Possibly related PRs
- jmrplens/gitlab-mcp-server#1517 — It adds pagination output metadata and request inventory rows that the R-PAGE audit evaluates.
Suggested reviewers: sourcery-ai
Merge Risk: 
Minimal · up to c22b6
-
No actionable issue is established for this change; it is mergeable after normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 inconclusive Passed checks: 4 passed-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 93.02% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 86 functions across 25 files. (4 skipped: 4 unsupported.)
-
Full details: Description check
-
Explanation
The description is thorough and closely follows the template's intent. It gives a detailed Changes section (schema 5 keyset record, the R-PAGE keyset half, the four problem kinds, and the new keyset-not-followable declarations), a detailed testing section with exact commands and results, and it names the related issue (Refs
#1165). It also explains the rebases and the SonarCloud fixes. Gaps: the template's 'Closes #' line is replaced by 'Refs#1165' (acceptable, since the template allows Refs). The 'Type of Change' checkboxes are not marked, and the description does not state a type. The checklist is not addressed item by item. Nothing in the description says whether the Breaking Changes section applies, though the change does alter the schema version and route counts for consumers. The 'How to Test' section is written as prose rather than numbered steps, but it does list concrete commands.Resolution
Mark the change type (for this change, likely New feature and Enhancement, plus Documentation update and CI / build / tooling if they apply). Confirm the related issue line in the template format, for example 'Refs
#1165' or 'Closes #'. Add a Breaking Changes / Migration Notes entry, or write N/A with a reason. The schema bump from version 4 to version 5 and the changed meaning of the routekeysetandpaginationfields may affect consumers of the record. Put the How to Test steps in a numbered list that a reviewer can follow. Confirm the checklist items in the template (for example, DCO sign-off, file headers, and documentation updates), either by checking them or by noting which ones are deferred to the top of the stack.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
GitLab reads pagination=keyset without declaring it, so the live record could not say which routes page by keyset: of the 53 whose code reaches a keyset paginator at v19.5.0-ee, four declare pagination and 22 declare no cursor either, because paginate_with_strategies, GitalyKeysetPager and the routes that write their own next link read both from params on their own. R-PAGE needs that answer to hold a list action's page_token to its route. Schema version 5 records it from the booted image. For every route, introspect.rb walks the instruction sequence of the route's block and of every helper of GitLab's own it calls on the endpoint, through the helpers Grape serves the route with, and records each place that calls a keyset paginator: paginate_with_strategies, GitalyKeysetPager, a finder executed with gitaly_pagination: true, a route writing its own next link or cursor headers through Keyset::HeaderBuilder, and keyset_paginate. Each site is located, quoted, named with the helpers the walk went through and the feature flags it is behind with their defaults, and carries the parameter its next page is asked by. Which parameter each library paginator names is not copied into the script: it has CursorPager, Keyset::Pager and GitalyKeysetPager each write a next link and reads the parameter back, into keyset_paginators. A site says where GitLab can serve a keyset page, not that it does. paginate_with_strategies picks CursorPager or Keyset::Pager by the model of the relation it is handed, and answers 405 to a model and order it does not support, and the walk reads code rather than the relation a request builds. So a paginate_with_strategies site carries CursorPager's cursor unless the call passes use_cursor: false, and on a model that declares no keyset orderings GitLab never reads it: the two service account token lists, which page PersonalAccessToken, are recorded with `cursor` and answer every keyset request with 405. The field's comments say so, and the reader that holds a route to whether it really pages by keyset declares such routes. -check, and the generator before it writes, refuse a version 5 record without that probe, which is what a version 4 script labeled 5 produces, one below the floor of 35 keyset routes, a site naming a paginator nothing reads, no location or no cursor, and, since the walk never follows a call on another object, a route that declares a cursor parameter and records no site asked by it. The source block counts the keyset routes and the count is held to the content. The comments that still called the record schema 4 now say 5. The record is re-recorded on gitlab/gitlab-ee:19.5.0-ee.0, the same digest, and differs from the schema 4 record only in its keyset half, the note and the provenance: entities, routes, features and both fine-grained halves are byte for byte what they were. It marks 53 routes. Of the ones this server's actions send, it marks every route the classification of issue 1165 found paging by keyset or refusing it. Each carries the cursor the classification found, except the two service account token lists, which the classification found refusing every keyset request and the record gives CursorPager's `cursor`. It marks two more the classification classed as ignored: the commit list, behind commits_keyset_pagination, which GitLab ships off, and GET /groups, whose keyset branch serves only an anonymous caller. The rest are routes no action sends, among them the work item REST routes, the MLflow searches and the internal Orbit routes. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
make coverage-mutants on cmd/gen_api_live reported one survivor and two mutants not covered, none of them a missing test. runnerDetail cut the runner's stderr to its last runnerStderrLines lines under a length test, and at exactly that many lines the cut and the whole are the same text, so `>` and `>=` cannot be told apart. bootTimeout and pollInterval multiplied in a package-level initializer, which sits outside every block a coverage profile records, so the run never tried the operator although the test of the shipped defaults reads both values. runnerDetail now cuts with max, which has no boundary to mutate, and the two defaults are parsed from "20m" and "10s". The values, the tail a failure reports and every test are unchanged; the package now reads Lived 0 and Not covered 0. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…yset R-PAGE asked whether a list says where it ends, and a keyset route raised no finding by design: the record's params could not say which routes GitLab pages by keyset, so nothing asked whether an action offers page_token where it means something, or whether a keyset page can be followed. The live record's schema 5 says where each route's code reaches a keyset paginator, and R-PAGE now reads it. The keyset half joins each action to its routes through the request record the handlers were derived into, at the action's own grain, and reports four problems: page_token offered where no route the action sends can be paged by keyset; a list on a keyset route that offers no page_token; an action offering it on a keyset route whose output carries no next_page_token; and one whose package was never recorded sending the route the parameter its next page is asked by. A feature flag GitLab ships off does not make a route offset, since an instance that turns it on, GitLab.com for the commit list, pages it by keyset. A route the record marks and GitLab serves this server no keyset page it can follow is declared in a new category, keyset-not-followable, with what GitLab's handler does at v19.5.0-ee: GET /groups, whose keyset branch only a signed-out caller reaches; the two service account token lists, which page PersonalAccessToken, a model with no keyset orderings, and answer every keyset request with 405 although the record gives them CursorPager's cursor; and a user's contributed projects, whose next link is never applied. A declared route is held to as offset, so an action offering page_token on it is reported, and a list sending it without page_token is declared. A declaration no finding uses fails the gate like every other stale declaration. The route counts read the record's keyset field instead of the params: keyset_routes goes from the 10 routes that declare per_page without page to the 53 whose code reaches a keyset paginator, and a route can count as offset as well. Against the committed record, 202 actions offer page_token and 20 of them on a keyset route, each publishing next_page_token and followed in the unit suite; 182 offer it on an offset route, which is the classification of the first layer of this stack, ignored and keyset not followed, without the commit list; and the three lists that send a keyset route without page_token, group.search and the two token lists, are declared. It reports and does not gate, as the collection half does. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
SonarCloud reported three findings on the keyset half of introspect.rb: next_link_of raised a bare string (S7815), scan had a cognitive complexity of 25 against a limit of 15 (S3776), and record_site took ten parameters against a limit of seven (S107). next_link_of now raises MissingNextLinkError, a StandardError, with the same message. scan keeps the loop over a sequence and hands each instruction to scan_instruction, which reads it in four steps of their own: open_guard, read_operand, read_flag_call and follow_call. What scan carried in locals from one instruction to the next is a ScanState, and record_site takes it with the call, the index and the flags. Nothing the walk records changes. Loaded beside the previous version over a fixture that reaches every paginator kind, flag guards of both recorded shapes and the shapes left unrecorded, helpers, nested blocks, a call to super and a recursive helper, it records the same sites in the same order, and the probe fails with the same message. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
bbc1d2d to
c22b625
Compare
…ion list The case in scan had no default branch, which SonarCloud reads as a missing case (rubydre:S131). An instruction list holds line numbers, labels and events, and instructions, so the default branch reads nothing; it now says so. The walk's output is unchanged: the equivalence harness finds the same 35 sites and the same error. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
|
…#1642) Refs #1595 Refs #1604 Refs #1605 Refs #1606 Refs #1607 Refs #1608 Refs #1609 Refs #1610 Refs #1611 Refs #1612 Refs #1613 Refs #1614 Refs #1615 Refs #1616 Refs #1617 Refs #1618 Refs #1209 Refs #1619 Refs #1620 Refs #1621 Refs #1622 Refs #1623 Refs #1624 Refs #1625 Refs #1626 Refs #1627 Refs #1628 Refs #1629 Refs #1630 Refs #1631 Refs #1632 Refs #1633 Refs #1634 This is the seventh layer of the auditor stack. The sixth added R-ROUTE and gated on stale declarations only, which left 730 routes of the 19.5.0 record that no action sends and nothing answered. This layer answers every one of them and turns on the full gate. ## What the check detects `-scope=routes` now fails on a finding of any class (a route nothing answers, an unknown skip reason, an internal route left in, a contradicted decision), as it already did on a declaration that no longer describes the tree. The gate reads those classes from `findingClasses`, so a class added there is gated without a second list to keep in step. A re-pin that brings a route, or an action that stops sending one, fails the gate until the change that causes it decides the route, and every family row pins how many routes it answers. One declaration problem is new: a `tracked` row over a route whose client-go method `acceptedMissingMethods` decides `COVERED_GENERIC` or `INTENTIONAL_SKIP_*`. The row says an action is still to implement and the decision that the route needs none, so one of the two no longer describes the tree. ## How the routes are answered I applied the decisions of 2026-10-27. `covered-by-graphql` is a REST route whose operation an action does over GraphQL, the action checked in the catalog and in the request record: 49 routes. They are the `/-/` epic routes and the child epic links, the five work item operations and a child's attachment and detachment behind `work_item_rest_api`, a group's work item's discussions, notes and children (13 routes the `group.epic_*` actions cover, since their GraphQL reads the group's work item by its iid whatever its type), a vulnerability's confirmation, dismissal, resolution, revert and read, and a pipeline's findings. A project's and a namespace's work item discussions, notes and children stay tracked, and so does the resolution of a discussion, which no action does. `client-protocol` is the protocol a package manager's or MLflow's own client speaks, 150 routes, except a download, which is tracked. The line is what the route serves, not how GitLab words its summary: what a package or a model version holds (one of its files, its archive, its source or a file inside it, such as a Go module's `go.mod`) is a download, while an index or a metadata document a client reads to find and resolve packages, and the URL or the location of the content, are the protocol's. So Debian's `Release` and `Packages` files, Helm's `index.yaml`, RPM's `repodata` and RubyGems' spec indexes and gemspecs are `client-protocol`. The witness holds every `client-protocol` route whose summary says download to a list that names what each serves instead, and every entry of that list and every protocol root to a route it answers. `tracked` is everything else, 534 routes, under 32 issues: one for each of thirty catalog domains and one for each of two domains the catalog does not hold yet, the virtual registries and GitLab Duo. The fifteen `public_endpoint` routes and every administrator, GitLab.com and tier route are among them, and a route's reason says it is public exactly when its skip reason is `public_endpoint` (`POST /glql` is not; `GET /glql/schema` is). Seventeen of the tracked routes were settled by `acceptedMissingMethods`: six `COVERED_GENERIC` (a group's and a project's direct members, a group's subgroups, the delete of an LDAP link by its `cn`, a merge request's changes and the `PUT` of the error tracking settings) and eleven `INTENTIONAL_SKIP_*` (nine downloads, the streamed repository archive and the deprecated write of a merge request's approvals). A route still to implement is no method R-ACTION may settle, so those entries leave the table and R-ACTION lists the seventeen methods as its backlog, 18 missing methods with `TerraformStates.List`, which was there already; `Orbit.GetDsl` was the nineteenth until the Orbit migration, and is decided `SUPERSEDED_UPSTREAM` now. The creation of a vulnerability keeps its `COVERED_GRAPHQL` service decision, which does not carry because no GraphQL action creates one. The two runner token routes and the integrations list carry no decision at all: they are tracked rows whose reason names `runner_token_auth` or the `/services/` twin. The table moved to `family_table.go`, written with five constructors (`tracked`, `download`, `publicEndpoint`, `protocol`, `overGraphQL`). It has 331 rows, more than the design estimated, because no two rows may answer one route: downloads sit inside the protocol trees, so each protocol is split around its downloads, and a group's work item routes are split from a project's and a namespace's. The Orbit rows are as the Orbit migration (#1585) left Orbit, all under #1209, the issue that already tracked the Orbit routes no action reaches. The five routes the `orbit.*` actions sent until GitLab.com removed them (`GET /orbit/status`, `/orbit/schema`, `/orbit/tools`, `/orbit/schema/dsl` and `/orbit/graph_status`) inherit `SUPERSEDED_UPSTREAM` from the layer below, and `POST /orbit/mcp`, which the actions send in their place, is reached, so the `/orbit/mcp` row tracks the endpoint's SSE listener alone, `GET /orbit/mcp`. `GET /orbit/agent/commands`, which `orbit.dsl` read until GitLab.com removed it too and which no client-go method sends, shares a row with the invocation of a command; the query templates, the named templates, the response format guidance and the skills stay tracked, the reasons of the two GitLab.com removed in favour of GraphQL saying so. ## Witness tests, red then green On the parent as it stood before the stack was rebased onto #1585 (9a1b315fb), with this layer's tests, a stand-in for the four identifiers they need (two issue constants, `publicTail` and `Summary.counters`) and without its table or gate: ```text $ go test -count=1 -run "TestSummary_Clean|TestJudgeAll_AFamilyRow|TestRun_Repository" ./cmd/audit_1to1/internal/routes/ --- FAIL: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration (0.00s) --- FAIL: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration/an_unanswered_route (0.00s) analyze_test.go:301: clean({... Unanswered:1 ...}) = true, want false (and the unknown skip reason, the two internal classes and the contradicted decision) --- FAIL: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther (0.00s) --- FAIL: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther/unanswered (0.00s) analyze_test.go:318: one route of class unanswered: clean() = true, want false (and the other four finding classes) --- FAIL: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange (0.02s) --- FAIL: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange/a_tracked_row_over_a_route_a_COVERED_GENERIC_decision_settles (0.00s) analyze_test.go:575: stale = [], want ["family GET /gizmos tracks GET /gizmos, and Gizmos.ListGizmos is decided COVERED_GENERIC, which says the route needs no action of its own: an action still to implement is no method acceptedMissingMethods settles"] (and the INTENTIONAL_SKIP_BINARY case, and the cases whose gate the parent passes with findings left) --- FAIL: TestRun_Repository_DeclaresTheRoutesADecisionDoesNotCarryOnto (0.22s) analyze_test.go:960: GET /groups/:id/members = {Route: Class: ...}, want it tracked by a family row whose reason names "", with the decisions "Groups.ListGroupMembers unadjudicated" (and the other 21 routes of the witness) --- FAIL: TestRun_Repository_HoldsTheDuoWorkflowRoutesToTheTokenThatCallsThem (0.16s) analyze_test.go:1008: composite identity = [...], 0 a user's token calls tracked by the Duo row and [... unanswered ...] otherwise, want [...], 14 and none --- FAIL: TestRun_Repository_AdjudicatesEveryRouteNoActionReaches (0.16s) analyze_test.go:1089: declared map[tracked:3] under 2 issues; want map[client-protocol:150 covered-by-graphql:49 tracked:535] under one issue for each of 30 catalog domains and of the virtual registries and GitLab Duo, which it does not hold yet analyze_test.go:1092: 0 public_endpoint routes tracked, want 15; ... analyze_test.go:1095: ...; roots and listed downloads no client-protocol route answers: ["/group/:id/-/packages/" "/groups/:id/-/packages/" "/packages/" ...] --- FAIL: TestRun_Repository_HoldsTheWorkItemRoutesToTheActionsThatDoTheirOperation (0.14s) analyze_test.go:1174: work item routes by scope and category = map[], want map[groups:map[covered-by-graphql:20 tracked:10] namespaces:map[covered-by-graphql:7 tracked:23] projects:map[covered-by-graphql:7 tracked:23]]; ... FAIL ``` Since the rebase the witness of the decisions that do not carry holds 21 routes, not 22, and the count it wants is 534 tracked: `GET /orbit/schema/dsl` is inherited now, with the other four Orbit routes GitLab.com removed. With this layer's gate check and its seventeen `acceptedMissingMethods` entries still in place, `TestRun_Repository_GateIsGreen` names the contradiction on the real tree, six `is decided COVERED_GENERIC`, nine `is decided INTENTIONAL_SKIP_BINARY` and two `is decided INTENTIONAL_SKIP_OTHER`: ```text analyze_test.go:837: the gate fails: 0 routes no action reaches are left findings, among them [], ...; and the stale declarations ["family * /groups/*/uploads/** tracks GET /groups/:id/uploads/:secret/:filename, and GroupMarkdownUploads.DownloadGroupMarkdownUploadBySecretAndFilename is decided INTENTIONAL_SKIP_BINARY, which says the route needs no action of its own: ..." ...] ``` Here: ```text --- PASS: TestSummary_Clean_PassesOnlyWithoutAFindingOrAStaleDeclaration (0.00s) --- PASS: TestSummary_Clean_FailsOnEveryFindingClassAndOnNoOther (0.00s) --- PASS: TestJudgeAll_AFamilyRow_AnswersItsRoutesUntilTheyChange (0.02s) --- PASS: TestRun_Repository_GateIsGreen (0.88s) --- PASS: TestRun_Repository_HoldsTheRoutesClientGoLaterModels (0.20s) --- PASS: TestRun_Repository_DeclaresTheRoutesADecisionDoesNotCarryOnto (0.22s) --- PASS: TestRun_Repository_HoldsTheDuoWorkflowRoutesToTheTokenThatCallsThem (0.27s) --- PASS: TestRun_Repository_AdjudicatesEveryRouteNoActionReaches (0.26s) --- PASS: TestRun_Repository_HoldsTheWorkItemRoutesToTheActionsThatDoTheirOperation (0.25s) --- PASS: TestDeclares_ATrackedRowOverARouteADecisionSettles_IsAProblem (0.00s) ``` The witness is sensitive to the version on purpose: the client-go bump and every re-pin move its counts, and that change is the evidence the triage was run again. ## On main 2137 routes: 1007 reached, 20 outside the API, 186 skipped, 94 by a slug, 49 aliases, 48 inherited and 733 declared by 331 rows, with no finding and no stale declaration. Each tracked route is in the text of its domain's issue with its row, its summary and the client-go methods that send it, if any. ## Checks `go test ./... -count=1` with the snapshot parity deferred, and the `audit_1to1` suites on the real tree without it. 100% statements and every condition both ways on `routes` (gobco 242 of 242) and on `actions` (22 of 22); gremlins Lived 0 and Not covered 0 on `routes` (181 killed) and on `actions` (16 killed). `golangci-lint` is clean on `cmd/audit_1to1/...`, and the `make analyze` steps the layer touches pass: the routes, params and paths gates, the sdk gate, the subtest, goroutine, unread-constant, closed-server, godoc and test-name gates, the documented tool names and `golangci-lint fmt`, as do markdownlint, the table formatter, the doc link check and the em dash check. `gen_testing_docs --check` is regenerated at the top of the stack, as below this layer. The two pieces of work this layer kept clear of have merged since, the keyset half of the live record (#1574) and merge request diff pagination (#1584), and so has the Orbit migration (#1585), which stopped sending five Orbit routes this layer counted reached (`GET /orbit/agent/commands`, `/orbit/graph_status`, `/orbit/schema`, `/orbit/status` and `/orbit/tools`) and started sending `POST /orbit/mcp`, which the `* /orbit/mcp` row tracked. The stack is rebased onto main after all three, the Orbit rows are redone as above, and every figure in this description is measured there except the red run, which is quoted from the parent before that rebase. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
… toolutil declares (#1643) Refs #1595 Refs #1603 This is layer A8 of the auditor stack (H7). The type grain of `-scope=paths` now holds every compared type one level down in both directions, and the R-OUTPUT field diff reads the converters `internal/toolutil` declares. Nothing new gates: what reaches the gate is still a declaration that answers nothing, now at either level. ## What it detects The nested comparison compared 44 nested types, all of them under the 68 compared types no struct names: `publishedTypesIn` filled `Nested` only for those, and a payload is named by its envelope, so the other 184 compared types, the merge request a list wraps among them, were never held one level down. That is how `head_pipeline.name` stayed published on every merge request although `API::Entities::Ci::Pipeline` sends no name. The walk now records what every type nests: 294 nested types compared. The package grain keeps reading one level down from a top-level type, since it joins on names alone and a name two levels down silenced nine of its findings. The nested level now asks the reverse too (`shapes.typed.nested_unsurfaced`): a key the entity under a property sends that the nested type does not publish, read on the entity the record's `using` names, key by key as the top level is, since two routes of a type can render one property with two entities, with `sdk_models` from the client-go struct under the same key (`OutputPairing.SDKNested`, and the same on projections). Both declaration tables gain `Under`, and a declaration answers a nested finding only when it names the property. One nested type under one property of two types of a package is reported once for each, so both finding lists now close their order on the client-go struct and the endpoints each finding was judged against: four runs of `-scope=paths` over this tree write the same bytes. The R-OUTPUT diff reads `internal/toolutil` and reports it as a `toolutil` entry. `toolutil.NewPipelineOutput` fills `head_pipeline` and no domain package declares a converter of it, so a key client-go adds to `gl.Pipeline` reached no finding. `CollectPairs` and `CollectOutputPairings` are unchanged, so R-ENUM and the type grain do not move; passing the shared pairs to R-ENUM is left unmeasured here. ## Witness tests `TestTypedShapeCheck_Repository_HoldsTheHeadPipelineToThePipelineEntity` (paths, real tree), red on the parent a81f7cd28, where its first commit's version, the `name` half, copied in on its own fails with `no nested finding for internal/tools/mergerequests head_pipeline.name, want toolutil.PipelineOutput's name reported unpublished`, and the same for `deploymentmergerequests`. Green here, with `head_pipeline.archived` reported on both, read on `API::Entities::Ci::Pipeline`, sent always and `sdk_models` false. The parent's report has no `nested_unsurfaced` and names no `archived` for a merge request. The bump turns `sdk_models` true, since `gl.Pipeline` models `archived` from v3.36.0. `TestBuildReport_Repository_ReportsTheConvertersOfTheSharedShapes` (structs, real tree), copied on its own into the parent, fails there with `package "toolutil" not found in report` (the parent's report has 153 packages). Green here: a `toolutil` entry exists, and `PipelineOutput` against `gl.Pipeline` carries the same keys in v3.30.0, three of them with a type of its own (`detailed_status`, `source` and `user`), which the bump turns into a missing `archived`. `TestBuildReport_Repository_PairsTheConvertersTheSharedShapesDeclare` checks that pair comes from the shared package's own converter, and `TestCollectOutputPairings_Repository_ReadsTheStructsAMergeRequestNests` pins `name` and no `archived` under `gl.MergeRequest`'s `head_pipeline`. Fixtures cover each rule: a list's payload has its part judged; the reverse is reported under its property and read on the entity that sends the key, also where two routes, or two routes merged into one shape, render the property with two entities; `sdk_models` reads the same property only; a projection answers it from its struct; a declaration with `Under` answers its finding, goes stale once the key is published, and is held to its evidence like a top-level one; the summary counts the nested level apart from the top; two types nesting one type report their findings in one order; the package grain does not credit what an inner type nests; and a stand-in `toolutil` converter of `gl.Part` reports `archived` missing once a stand-in SDK adds it to `Part`, while the SDK as it stands leaves the pair without a gap. ## Findings on main and how each is answered `head_pipeline.name` and `head_pipeline.archived` on `mergerequests` and `deploymentmergerequests` are the witness cases; `archived` is published in the client-go bump stack, which models it. The nested level reports 111 published keys GitLab does not send under their property and 1085 keys it sends that the nested type does not publish, 642 of them not modeled by client-go either. None gates, and none is declared here: the design leaves the nested findings beyond `archived` to B4 or to an issue, so they are listed by nested type in the follow-up. `summary.typed_undeclared_fields` reads 111, all at the nested level. R-OUTPUT gains 42 output pairs with 29 missing and 20 extra fields and 17 type mismatches under `toolutil`, reported like the domain packages' 387 missing and 85 extra fields and 593 type mismatches, and one more package with gaps (60, from 59). No declaration table goes stale. ## Checks All green: `go test ./... -count=1` with `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred`, the audit_1to1 tests on each commit, golangci-lint, markdownlint, the doc link check, `format_md_tables --check`, `make check-test-file-names`, `make check-em-dash` and `make check-pr-description`, and `make analyze` steps 2, 4, 7, 8, 14, 32 and 33, plus `-scope=sdk`, `-scope=structs` and `-scope=enums`. Four runs of `-scope=paths` over the head write the same bytes. Statement coverage 100% in `paths` and `structs`; conditions 1060/1060 and 728/728 (gobco). Mutants: `structs` 279 killed, 0 lived, 0 not covered (the 2 timed out are in `cross_call.go`, which this layer does not touch); `paths`, limited to the files this layer changes, 286 killed, 0 lived, and 420 not covered, every one an `ARITHMETIC_BASE` `+` in a package-level const or var initializer of `sent_declarations.go` (331), `shape_declarations.go` (85) or `published.go` (4), which Go's coverage does not instrument, and none on a changed line. ## Overlaps #1584, which carried #1517 into main, added two entries to `declaredUnsurfaced` for the merge request the two note packages read to check an inline position. This layer adds the `Under` field and changes no entry, so an entry without `Under` keeps its meaning, and the rebase onto main was mechanical: the conflicts were the import block of a test file and a comment of `shapes.go`, both of which #1574 had changed. No `apilive` or `gen_api_live` file is touched. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>



Refs #1165
This is the second layer of the first #1165 stack, the keyset oracle the plan's question 27 settled on and the rule that reads it. The live GitLab record (
docs/development/gitlab-api-live.json, written bycmd/gen_api_live) now says, for every route, where the code it reaches calls one of GitLab's keyset paginators and which parameter its next page is asked by, and it is re-recorded on 19.5.0 at schema version 5. R-PAGE reads it: it holds every action'spage_tokento the routes the action sends, and every list GitLab can page by keyset to thenext_page_tokenit is followed by. Nothing in the record could give that answer before, because GitLab readspagination=keysetwithout declaring it: of the 53 routes whose code reaches a keyset paginator atv19.5.0-ee, four declarepaginationand 22 declare no cursor either.What the record says now
Each route whose code reaches a keyset paginator carries a
keysetlist, one entry per place its code calls one. An entry names the paginator, the file and line of the call with the line quoted, the helpers the walk went through to reach it (present_projects,render_work_items_collection_for), the parameters the next page is asked by, and the feature flags the call is behind with their defaults. A route withoutkeysetreaches no keyset paginator, so GitLab answers it with an offset page whateverpaginationsays; in schema 4 the same absence meant "not recorded", which is why the version moved, the same inversion every earlier bump guarded against.A site says where GitLab can serve a keyset page, not that it does.
paginate_with_strategiespicksCursorPagerorKeyset::Pagerby the model of the relation it is handed and answers 405 to a model and order it does not support, and the walk reads code, not the relation a request builds. So apaginate_with_strategiessite carriesCursorPager'scursorunless the call passesuse_cursor: false, which assumes a model with keyset orderings. The two service account token lists pagePersonalAccessToken, which declares none: the record gives themcursor, and GitLab answers every keyset request there with 405 and never reads it. The comments of the field, ofapilive.KeysetSiteand of the script say so, and R-PAGE declares those routes rather than trusting the site (below).The five paginators the walk recognizes are
paginate_with_strategies,GitalyKeysetPager, a finder executed withgitaly_pagination: true(the internal Orbit commit list), a route writing its own next link or cursor headers throughGitlab::Pagination::Keyset::HeaderBuilder(the instance audit events, the registry tags, the work item discussions), andkeyset_paginate(the work item discussion notes and the MLflow searches).The record also carries
keyset_paginators, which parameter each of GitLab's library paginators names in its next link. I did not copy those names into the script: it hasCursorPager,Keyset::PagerandGitalyKeysetPagereach write a next link through the method GitLab calls after serving a page, and reads the parameter back (cursor,id_afterandid_before,page_token, andcursorfor theX-NEXT-CURSORheader). A release that renames one changes the record rather than leaving it wrong. The source block counts the keyset routes, and the gate holds the count to the content like the other figures.How the walk works
introspect.rbtakes each route's block from the endpoint Grape built, compiles the helpers it is served with the way Grape does when it mounts the endpoint (a runner never mounts one), and reads the instruction sequence of the block and of every helper of GitLab's own it calls on the endpoint, followingsuperinto the CE method an EE module overrides. It records a site at a call to one of the paginators above; the cursor ofpaginate_with_strategiesisCursorPager's unless the call passesuse_cursor: false, the cursor of a route's own next link is the key of the hash it hands the builder, and the cursor ofkeyset_paginateis the param it is handed. A feature flag is recorded on the sites it guards when it is tested asif Feature.enabled?(:flag)orunless Feature.disabled?(:flag), which reads unambiguously from the branch instructions; that is how the commit list comes out behindcommits_keyset_pagination, shipped off. The walk takes about forty seconds inside the container.The walk follows calls on the endpoint, never calls on a finder or a service, so a route a service pages by keyset on its own would read as paged by offset. That is what the new gate rule is for: a route declaring a parameter the record knows as a cursor is one that says it pages by keyset in its own words, and if it records no site, or no site asked by that parameter, the walk and GitLab disagree. All 31 routes that declare a cursor at 19.5.0 carry a site asked by it.
What the gate refuses
make check-api-live, and the generator before it writes, refuse a version 5 record withoutkeyset_paginators(what a version 4 script labeled 5 produces), one below a floor of 35 keyset routes, a site naming a paginator nothing reads, a site with no file and line or no cursor, a probe naming no parameter or one nothing reads, and a route declaring a cursor the record does not follow. A passing run prints one more line, how many routes reach a keyset paginator and how many only behind a flag GitLab ships off.The re-record
I booted
gitlab/gitlab-ee:19.5.0-ee.0alone on the remote Docker host, the digest the record already names (sha256:f04c71f565601283971d6472dd466609f4679450526f39aab3e3dcd6d23dc0ec), withgo run ./cmd/gen_api_live/ -image gitlab/gitlab-ee:19.5.0-ee.0. Apart from the keyset half, the note, the schema version and the provenance, the record is what it was: entities, routes, features and both fine-grained halves compare equal underjq -S, and the source figures are unchanged.The record marks 53 routes. Of the routes this server's actions send, it marks every one the first layer's classification found paging by keyset or refusing it (the 13 that follow it, the 5 refused by ordering, the 2 refused always and
project.list_user_contributed). Each carries the cursor the classification found,laston the registry tags andid_after/id_beforeon the project lists included, except the two service account token lists, which the classification found refusing every keyset request and the record givesCursorPager'scursor, for the reason above. Two of the routes it marks are ones the classification called ignored, and every other route it marks is one no action of ours sends: the work item REST routes, the MLflow searches, the internal Orbit routes,GET /pipelines, the package pipelines and the admin data management list. Of the two, the commit list (repository.commit_list,repository.file_history) reachesGitalyKeysetPagerbehindcommits_keyset_pagination, which the record says is off by default, so it agrees with the classification on a default instance.GET /groupsreachespaginate_with_strategiesonly for an anonymous caller (present_groups_with_pagination_strategiesreturns early for a signed-in one); the record names the helper but not that condition, since an early return is not a shape the walk reads, and this server always signs in.R-PAGE reads it
R-PAGE asked whether a list says where it ends, and a keyset route raised no finding by design, because the params could not say which routes GitLab pages by keyset. Its new keyset half (
pagination.keysetin the report,cmd/audit_1to1/internal/paths/pagination_keyset.go) joins each action to the REST routes it sends throughdocs/development/action-requests.json, so it judges at the action's own grain, and reports four problems:page-token-on-offset-route: the action offerspage_tokenand no route it sends can be paged by keyset, so GitLab ignores the parameter or refuses it.keyset-route-without-page-token: a read whose output is a list sends a route GitLab can page by keyset and does not offerpage_token.keyset-route-without-next-page-token: the action offerspage_tokenon a keyset route and its output carries nonext_page_token.keyset-route-never-followed: the action offerspage_tokenon a keyset route and no package serving it (its owner, or the package of a handler or site the request record names) was recorded sending the route one of the parameters its next page is asked by. This one is at the inventory's package grain, which the half'sgrainsays.A site behind a flag GitLab ships off counts as keyset: an instance that turns the flag on, GitLab.com for the commit list, pages the route by keyset, and the action's
page_tokenis what reaches it there.A route the record marks and GitLab serves this server no keyset page it can follow is declared in a new category,
keyset-not-followable(declaredKeysetRoutesinpagination_declarations.go), each entry with what GitLab's handler does atv19.5.0-ee, by file and line:GET /groups: only an anonymous request reaches the keyset branch (lib/api/groups.rb:171).GET /groups/:id/service_accounts/:user_id/personal_access_tokensandGET /projects/:id/service_accounts/:user_id/personal_access_tokens:PersonalAccessTokenhas no keyset orderings, so every keyset request is answered 405 (lib/api/helpers/pagination_strategies.rb:30-43).GET /users/:user_id/contributed_projects: the finder is built without params, so the next link GitLab writes is never applied (upstream-bugs row 112).A declared route is held to as offset: an action offering
page_tokenon one is reported, with the route in itsnot_followable, and a list sending one withoutpage_tokenis declared. A declaration no finding uses is stale and fails the gate, like every other declaration table of R-PATH; the findings themselves report and do not gate, as the collection half's do.The route counts read the record's keyset field instead of the params:
keyset_routesgoes from the 10 routes that declareper_pagewithoutpageto the 53 whose code reaches a keyset paginator, and a route can count as offset as well. Against the committed record, 202 actions offerpage_token. 20 of them offer it on a keyset route, and every one publishesnext_page_tokenand was followed in the unit suite. 182 offer it on an offset route, which is exactly the first layer's classification of ignored and keyset-not-followed actions without the commit list. The three lists that send a keyset route withoutpage_token,group.searchand the two token lists, are declared. Takingpage_tokenout of those 182 inputs is the next stack of #1165.Also in this layer
A second commit leaves
cmd/gen_api_livewith no mutant the mutation run cannot judge.runnerDetailcut the runner's stderr under a length test whose boundary is an equivalent mutant (at exactly the limit the cut and the whole are the same text), and the two boot defaults multiplied in package-level initializers, which no coverage profile records. The tail is now cut withmaxand the defaults are parsed from"20m"and"10s"; the values and the behavior are unchanged.How I tested it
I wrote and measured this layer on the first layer as it stood over
e3e09d290, and then rebased the stack twice: ontomainat6c9066aef, after the OpenSSF Best Practices stack, and ontomainat56758fb71, after the access level and edition tier stack (#1567 to #1571). The rebases changed nothing in this layer's lines. In the Go and Ruby files this layer touches,mainchanged only the copyright and SPDX header at their top; what it changed in theMakefile,docs/development/cmd-utilities.mdanddocs/development/request-paths.mdlies outside the lines this layer edits there, and merged without a conflict. The six files this layer adds (keyset.goand its test incmd/gen_api_liveand incmd/internal/apilive,pagination_keyset.goand its test incmd/audit_1to1/internal/paths) now open with that header too, written bymake fix-file-headersinto the commit that adds each. What I ran again after the rebases is at the end of this section.The three generator tests the plan asked for came first and were red on the stack's first layer,
a35673d89before the rebases, compiled against it unchanged:TestRunGenerate_ARouteCarryingAKeysetPaginator_ComesOutMarked(the route came out withkeyset = <nil>, and so did the probe and the count),TestRunCheck_ARouteDeclaringACursorTheRecordDoesNotFollow_IsRefusedandTestRunCheck_AVersion4RecordLabeled5_IsRefused(the parent refused both records only asschema version 5 and this build reads version 4). They edit the fixtures as JSON so they mean the same thing on a build with no keyset field, and they pass on this branch. Every rule has a case of its own inTestKeysetProblems_EachRule_NamesWhatItRefuses, with the whole sentence asserted, the floor at its edge and the shapes 19.5.0 records held to pass. The order of the generator's refusals has a case too: with the fine-grained and keyset refusals swapped, the second case ofTestRunGenerate_AKeysetSiteNothingReads_IsRefusedAfterTheFineGrainedHalffails.The R-PAGE cases read the keyset half as JSON, the way a reader of the report does, and were red on the parent commit,
1ece15937before the rebases (the pagination check carries no keyset half, and the route counts read off the params), compiled against it unchanged; the cases of the declaration table are new code and could not compile there.TestRun_TheRealTree_PassesItsOwnGatenow also holds the summary's keyset figures to the findings they count.cmd/audit_1to1/internal/paths,cmd/gen_api_live,cmd/internal/apiliveandcmd/gen_action_grants/internal/joinare at 100% statement coverage, gobco reads 1114/1114, 254/254, 186/186 and 304/304 conditions, andmake coverage-mutantsreads Killed 564, 216, 102 and 156, with Lived 0 and Timed out 0 on all four and Not covered 0 on the last three.cmd/audit_1to1/internal/pathsalso reports 483 mutants not covered, and every one is anARITHMETIC_BASEon a+between string literals in a package-level initializer, on a line this layer does not change: the declaration tables ofsent_declarations.go,shape_declarations.go,endpoint_declarations.go,orbit_declarations.goandpagination_declarations.go, a constant and a map ofpublished.go, and a regular expression ofendpoints.go. A-there does not compile, which is the tool artifactdocs/development/testing/README.mddescribes; the new declaration table writes each reason as one literal for that reason, which took out the 14 it would have added. Before the rebase,make analyzepassed every step but markdownlint, which could not start on the remote runner (npx: not found), and the same markdownlint command passed when run locally on the tree.make check-api-live,make check-action-grants-derivation,make check-action-grants,make audit-1to1-grants,make audit-1to1-paths,make check-e2e-static,make check-tenancy,make check-test-file-namesandgo run ./cmd/format_md_tables/ --checkpassed on the head,node scripts/check-doc-links.mjspassed,GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred go test ./... -count=1passed on the head (294 packages), and the e2e packages compiled.After the second rebase, on this head:
GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred go test ./... -count=1passes (301 packages),golangci-lint run --build-tags e2eandgolangci-lint fmt --diffpass on the four packages, the e2e packages compile, andmake check-api-live,make check-action-grants-derivation,make check-action-grants,make audit-1to1-grants,make audit-1to1-paths,make check-e2e-static,make check-tenancy,make check-test-file-namesandmake check-md-tablespass. R-PAGE's figures are the ones above: 53 keyset routes, 202 actions offeringpage_token, 20 of them on keyset routes, 182 on offset routes and 3 lists declared.make check-file-headersandmake check-dcoagainst the first layer pass. I did not measure coverage, conditions or mutants again, since the rebases changed no statement of the four packages, and the SonarCloud commit below changes only the Ruby script, which no Go coverage profile reads.SonarCloud
SonarCloud reported three findings on
cmd/gen_api_live/introspect.rbin this pull request, and a fourth commit answers them without changing what the script records:rubydre:S7815, araiseof a bare string innext_link_of: it now raisesMissingNextLinkError, aStandardErrorof its own, with the same message.rubydre:S3776,KeysetWalk#scanat a cognitive complexity of 25 against 15:scankeeps the loop over a sequence and hands each instruction toscan_instruction, which reads it in four steps of their own (open_guard,read_operand,read_flag_callandfollow_call), and what it carried in locals from one instruction to the next is aScanState.rubydre:S107,record_sitewith ten parameters against seven: it takes theScanState, the call, the index and the flags.The script only runs inside a booted GitLab, so I held the new walk to the old one directly: a harness loads the keyset half of both versions under one Ruby and walks a fixture whose methods reach every paginator kind the walk recognizes, flag guards of both recorded shapes and the shapes it leaves unrecorded (an early return,
if Feature.disabled?andunless Feature.enabled?), nested blocks, helpers, a call tosuperand a recursive helper. Over five entry points both versions record the same 35 sites in the same order, with the samevia, cursors and flags, and the probe fails with the same message.go test ./cmd/gen_api_live/ ./cmd/internal/apilive/andmake check-api-livepass, and the record does not change, since it holds a digest of what the script answered and not of the script.On that commit SonarCloud then reported
rubydre:S131, thecaseinscanwith no default branch. A fifth commit gives it one that reads nothing, with a comment saying why: an instruction list holds line numbers, labels and events, and instructions. The harness finds the same 35 sites and the same error, and the two checks above pass again.What the top of this stack does
The contributor documentation of schema 5 and of R-PAGE's keyset half (the
gen_api_livesection and the R-PAGE paragraph ofdocs/development/cmd-utilities.md, thecheck-api-liverow ofdocs/development/static-analysis.md, the R-PAGE paragraph ofdocs/development/request-paths.mdand its line inCLAUDE.md) anddocs/development/testing/testing.mdare refreshed at the top of this stack, with the rest of its regeneration. This layer regenerates no committed artifact, sogo run ./cmd/gen_testing_docs/ --checkfails here on the new tests, as theFRESHNESSdeferral below a stack's top expects.#1517
Nothing here overlaps #1517, which changes neither the record, its generator nor R-PAGE, and no file of this layer is one #1517 changes. The merge request diffs route it pages is one the record marks as offset, as the review of #1517 expected, so R-PAGE reports
mr_review.changes_getamong the 182 actions offeringpage_tokenon an offset route; taking it out of that input is #1530, which waits for #1517. Whichever of #1517 and this stack lands second regenerates the artifacts both change.