Repository navigation
Describe the live record's keyset half and R-PAGE's, cover the token lists end to end, and regenerate the stack's artifacts - #1575
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 11 minutes. View limit details
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 |
|
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters |
jmrplens
changed the title
jmrplens
force-pushed
the
b-h4b-1-regen
branch
from
October 28, 2026 15:26
edf00b1 to
c5ae50e
Compare
The token lists of a group's and of a project's service account no longer take pagination, page_token or order_by, since GitLab pages their PersonalAccessToken relation with paginate_with_strategies, answers every keyset request with 405, ignores page_token on an offset page and declares no order_by. Nothing drove either list against a real GitLab with any of those arguments, or with a sort order at all. Two scenarios in keyset_pagination_test.go give a service account three tokens, named so they sort in the order they were made, and on every surface send each of the three arguments, held to a refusal made before any request: the SDK's on the individual surface, whose schema admits no property it does not list, execute's list of the params it does not know on the dynamic surface, and the meta dispatcher's unknown field. They then follow the list one token a page by page number in name_asc order, holding each page to carry no next_page_token and to name the next page while has_more holds, and read it whole in name_desc order, so an order GitLab ignored reads as a failure rather than as a listing. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
The two layers below recorded where each route can be paged by keyset in schema 5 of the live record and gave R-PAGE a keyset half that reads it, and both left their contributor pages describing schema 4 and an R-PAGE that raises no keyset finding by design. The gen_api_live section of cmd-utilities.md now says what the walk records and how, why a site says where GitLab can serve a keyset page rather than that it does, and what -check refuses of that half; static-analysis.md and the repository map say the same in a line. The R-PAGE paragraph of cmd-utilities.md and its bullet in request-paths.md read the keyset routes off the record's keyset field rather than off the params, and describe the keyset half: its four problems and their two grains, the keyset-not-followable declarations and the figures against the committed record, 202 actions offering page_token, 20 of them on keyset routes, 182 on offset routes. The R-PAGE clause of CLAUDE.md names the rule and nothing more. The output format page, in English and Spanish, says that the two service account token lists, where GitLab refuses every keyset request, no longer take pagination or page_token and page with page, and that the lists still refusing keyset with 405 are the ones given an order GitLab cannot page by keyset. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
The layers below regenerate nothing, under the freshness deferral, and the two service account token lists of the first layer changed the surface every generated artifact describes: their inputs no longer offer pagination, page_token or order_by, and their sort enum names the eight orders the routes document instead of asc and desc. The tool snapshots of the individual surface and of the compact and full meta schemas change on exactly those two tools and the gitlab_group and gitlab_project schemas that carry them; the curated tools_meta.json does not change. The per-domain tool reference of the group and project groups, in English and Spanish, the llms files, the token footprint with the README block it writes, and the testing reference, refreshed with -skip-coverage, follow. The request inventory was recorded again on the whole unit suite and merged: the two token routes are no longer recorded sending order_by, page_token or pagination, and the group one is recorded sending page and per_page. The action request record and the fine-grained table, the site stats, the brand assets, the manifests and the e2e coverage page regenerate unchanged. Refs #1165 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
jmrplens
force-pushed
the
b-h4b-1-regen
branch
from
October 28, 2026 16:15
c5ae50e to
c7cc5c0
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1165
This is the top of the first #1165 stack. It documents what the two layers below added, the keyset half of the live GitLab record at schema 5 and the keyset half of R-PAGE that reads it, adds the end-to-end scenario for the two service account token lists the first layer changed, and regenerates once every committed artifact the stack moves, which the layers below leave stale under the freshness deferral.
The documentation
The contributor pages described schema 4 and an R-PAGE that raises no keyset finding by design. In
docs/development/cmd-utilities.md, thegen_api_livesection now says what the walk records for each route (the paginator, the file and line quoted, the helpers it went through, the parameters the next page is asked by, the feature flags with their defaults), why a site says where GitLab can serve a keyset page rather than that it does, and what-checkrefuses of that half. The R-PAGE paragraph readskeyset_routesoff the record'skeysetfield (53 routes, 39 of them offset too) rather than off the params (the 10 that declareper_pagewithoutpage), and a new paragraph describes the keyset half: its four problems and their two grains, thekeyset-not-followabledeclarations, and the figures against the committed record (202 actions offeringpage_token, 20 of them on keyset routes that publishnext_page_tokenand were seen following it, 182 on routes GitLab pages by offset alone, three lists declared).docs/development/request-paths.mdsays the same in its R-PAGE bullet,static-analysis.mdand the repository map in a line each, and the R-PAGE clause ofCLAUDE.mdnames the rule and nothing more.On the site, the output format page, in English and Spanish, says that the two service account token lists, where GitLab refuses every keyset request, no longer take
paginationorpage_token, refuse either before sending anything and page withpage, and that the lists still answering a keyset request with 405 are the ones given an order GitLab cannot page by keyset.The end-to-end scenario
Nothing drove either token list against a real GitLab with a keyset argument, or with a sort order at all. Two scenarios in
test/e2e/gitlab/common/keyset_pagination_test.go,TestKeysetPagination_GroupServiceAccountTokens_PageByOffsetAloneandTestKeysetPagination_ProjectServiceAccountTokens_PageByOffsetAlone, give a service account three tokens named so they sort in the order they were made, and on every surface:pagination: "keyset",page_tokenandorder_by, each held to a refusal made before any request: the SDK's on the individual surface, whose schema admits no property it does not list, execute'sUnknown paramson the dynamic surface, and the meta dispatcher'sunknown field;name_ascorder, holding every page to carry nonext_page_tokenand to name the next page whilehas_moreholds;name_descorder, so an order GitLab ignored, which leaves the tokens unsorted, reads as a failure rather than as a listing.Both were red on the parent: driving the server built from
e3e09d290, the base of the stack before it was rebased ontomain, against CE 19.5.0, every surface failed at the first argument (8 tests, 8 failures), because that server still sendspagination=keysetand GitLab answers it,405 {error: Keyset pagination is not yet available for this type of request}:refused as tool_error, and the test expected invalid_paramson the individual surface andthe error does not mention "unknown field \"pagination\""on the other two. That run was of the scenarios' first version, whose dynamic case expected the meta wording; the complete CE run of this stack showed that execute refuses the argument earlier, withUnknown params: pagination.and the list of valid params, and the dynamic case now expects that. With the final version, the red run on the parent fails again on every surface: GitLab answers405 {error: Keyset pagination is not yet available for this type of request}for bothgroup.service_account_pat_listandproject.service_account_pat_list. On the stack's head before the rebases, the green run of the two scenarios passes.The regeneration
One commit, with the comparison checked:
gitlab_groupandgitlab_projectschemas that carry them (pagination,page_tokenandorder_bygone,sortnaming the eight documented orders); the curatedtools_meta.jsondoes not change;-skip-coverage;order_by,page_tokenorpagination, and the group one is now recorded sendingpageandper_page.The action request record and the fine-grained table, the site stats, the brand assets, the manifests and the e2e coverage page regenerate unchanged.
How I tested it
The stack was written on
e3e09d290and rebased twice: ontomainat6c9066aef, after the OpenSSF Best Practices stack, and ontomainat56758fb71, after the access level and edition tier stack (#1567 to #1571). In this layer's two hand-written commits the first rebase changed one place:mainadded rows to the pipe table ofdocs/development/static-analysis.mdand padded it again, so I tookmain's table, put this layer's sentence back into thecheck-api-liverow and rango run ./cmd/format_md_tables/. The second changed nothing in them: whatmainchanged inCLAUDE.md,docs/development/cmd-utilities.mdanddocs/development/repository-map.mdlies outside the lines this layer edits there, and merged without a conflict. Both times the regeneration commit was not carried over: I dropped it and regenerated every artifact on the rebased tree. It changes the same 15 files as before. The tool snapshots, the tool reference, the llms files and the request inventory, recorded again on the whole rebased unit suite, change line for line as they did on the old base. The token footprint with its README block and the testing reference move by the same amounts frommain's new figures: 225 fewer tokens on each tier's individual surface, and 37 more test functions and three more test files undercmd/, the stack's own tests.With the freshness checks on, on this head after the second rebase:
go test ./... -count=1passes (301 packages), and again withGITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred.make analyzepasses 30 of its 32 steps on the remote runner, and the other two pass locally on the same tree: markdownlint (step 6, which needs Node) and the copyright and license headers (step 32, which reads git's file list). Every check the "Generated artifacts" job of CI runs passes, among themcheck-tool-reference,check-llms,check-footprint,check-site-stats,check-testing-docs,check-request-inventory,check-api-live,audit-1to1-paths,check-action-grants,check-e2e-static,check-tenancyandcheck-file-headers, withcheck-openpluginandcheck-server-json-packagesrun locally since they need the network.make check-dcopasses for the ten commits of the stack (the tenth is the second layer's answer to SonarCloud'srubydre:S131, which changes no recorded output).make test-e2e-http(780 tests, 8 skipped because the runner has no Docker) andmake test-e2e-stdio(277 tests) pass, the e2e packages compile, andtest/e2e/gitlab/commonpassesgolangci-lint runandgolangci-lint fmt --diff. The site passesi18n:check,facts:check,chips:check,llms:check,buildandlint, andnode scripts/check-doc-links.mjspasses.Against Docker GitLab 19.5.0, the complete CE run (
make test-e2e-cewith the skip gate) ran on this stack before the dynamic case of the two new scenarios was corrected: 1313 tests, and the only failures were those two dynamic cases, for the wording above. Its skip gate also failed on four skips, the Bitbucket Cloud and GitHub imports, because that worktree had no.envwith their credentials, which the next run has. The correction touches only that test file. On the stack's head before the rebases (25b20c981), with the skip gate on, the complete CE run passes 1313 tests (1 skip, which is declared) and the complete EE run passes 1642 tests (no skips).I did not run the Docker suites again after the rebases. Since that run, the server code this stack changes is the same apart from the comment that names the register row (now 143) and the file headers, and the Ruby script the second layer's SonarCloud commit splits, which only
gen_api_liveruns, inside a GitLab it boots. Whatmainbrought that those suites exercise is the dependency update of #1564 (golang.org/x/net0.62.0,google.golang.org/grpc1.85.0,gqlparser2.5.64 andtiktoken-go0.10.0 among them, client-go and the go-sdk unchanged), the Bitbucket Server fixture moved from 9.6 to 10.5 with basic authentication forced back on, which the import scenarios use, the copyright and SPDX header in every source file, which is comments, and the access level and edition tier stack: the access levels each write offers, cut to what its route accepts, a pending access request no longer reported as approved, the two checksum filters gone from the project lists andnew_epicfrom a project's notification settings, and scenarios of its own inaccessrequests_test.goandaccesstokens_test.go. The plaintext listener warning of #1561 is HTTP mode only, and the e2e suites drive stdio. None of it touches the two token lists, their packages or the scenarios this stack adds.#1517
#1517 is open, and the maintainer has asked for changes before merging it. This stack leaves to it everything it does or was asked to do: the offset paging of the merge request diffs in
mr_review.changes_get, with the pagination block its output gains and without keyset continuation, the page walk of the inline comment pre-check under RQB-013 with its check of the position's SHAs, and the curated text, request declarations and test moves the review asked for. R-PAGE classifiesmr_review.changes_getwith the rest, among the 182 actions offeringpage_tokenon an offset route, no declaration of it names that action, and nothing here changes it. Its unusedorder_by,sort,paginationandpage_tokenare #1530 and the pre-check'sposition_typeis #1531, both after #1517. The two share no hand-written file: this stack changescmd/audit_1to1/internal/structs/analyze.go, thegroupserviceaccountsandprojectserviceaccountspackages, R-PAGE incmd/audit_1to1/internal/paths(analyze.go,pagination*.go,shapes.go) and the live record's generator, and #1517 changessent_declarations.goin the same package, themr*packages,internal/toolutil/diff_position.goandinternal/tenancy, so a rebase of either is mechanical. Both change generated files:internal/tools/testdata/tools_individual.json,tools_meta_compact.jsonandtools_meta_full.json,docs/development/request-inventory.json,docs/development/token-footprint.mdandsite/src/data/token-footprint.jsonwith the README token block,docs/development/testing/testing.md,llms-full.txtandllms-full-individual-tools.txt. Whichever of #1517 and this stack lands second regenerates those files on top of the other, together with the ones only #1517 changes and this stack regenerates unchanged (action-requests.json, the fine-grained table,tools_meta.json,llms-full-meta-tools.txtand themr-reviewtool reference).