Repository navigation
Offer only the access levels each GitLab route accepts, and stop reporting a pending access request as approved - #1567
Conversation
…ccepts GitLab holds every membership access level parameter to a set its route or its model declares, and our schemas wrote theirs by hand per field. They offered 0, which no route accepts, 5 on a project, where no edition accepts it, and 60 on the list filters, which refuse it (issue 1280). toolutil.AccessLevelSet is that table, read from v19.5.0-ee: the filters, the project writes and the access tokens take Guest to Owner, the group writes take Minimal access as well, only on a top-level group whose license includes it, an LDAP group link takes Minimal access on any group, a broadcast message cannot target Security Manager, and a custom role extends Guest to Maintainer. Planner and Security Manager are in every set that takes the whole range, since Community Edition grants both. A set says whether it accepts a level, renders its levels as N=Role, and publishes itself as an integer enum through a schema override. min_access_level and shared_min_access_level get the filter levels as their enum centrally, as the canonical string enums are, because every route that takes them declares Gitlab::Access.all_values. An action's own override still wins. The R-ENUM exemptions for those filters cite the routes rather than the API pages, which listed 5, and excuse the 5 and 60 the enum leaves out. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…ead back a Minimal access approval access.approve_project and access.approve_group offered 0=No access and then sent no level for it, so GitLab applied its default and the requester became a Developer. Given a level GitLab does not grant there, 5 on a project or on a group that is not a top-level group of a licensed instance, GitLab answers the approval with 201 and the requester while the request stays pending: the approval service sets the level on the request, the save fails the member's validation, and the route presents the unsaved record. Both actions reported that as an approval (issue 1280). access_level is a *int now, so leaving it out and sending 0 are two calls. Left out, no level is sent and GitLab grants Developer. A level the scope does not grant, 0 and 60 on both and 5 on a project, is refused before any request, naming the levels the scope grants. The schemas publish those levels as their enum. Minimal access is the one level a group grants in some places and not in others, and the approval's answer is the same either way. On the Free tier it is refused without a request. Otherwise the group is read first and a subgroup is refused before the approval, and after the approval the requester is read back among the group's members: a requester who is not there is answered as a request still pending, and a read that fails says the outcome is not known. Both reads are marked optional for the fine-grained derivation, and the member read goes out under the caller's context rather than the approval's capture, so the membership reported is the approval's own. Each handler still builds ApproveAccessRequestOptions from its own input, so the 1:1 audit keeps pairing both inputs with client-go's options, and the R-ENUM exemptions name the levels each refuses. The group approval joins the actions whose Premium wording describes one level rather than the action. The common end-to-end scenario refuses 0 and 5 on a project and 0 on a group on every surface and finds each request still pending, then approves with no level and finds a Developer; the CE one refuses Minimal access on the Free tier, and the EE one refuses it on a subgroup and grants it on a licensed top-level group. The end-to-end file counts in CLAUDE.md and the repository map include the two new files, and the common package's count, one short since an earlier change added a file, is corrected with them. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
Every membership access level parameter now publishes the set its route accepts, from toolutil.AccessLevelSet, as an integer enum and as an N=Role list in its description, and the handlers' hints and the parameter guidance read the same table (issue 1280): - the eleven list filters stop offering 5 (and six of them 60); - the project member add and edit, the project share and the project invitation stop offering 5; - both invitations, the SAML group link and the LDAP group link stop offering 0, and the group member edit and the invitations' hint stop offering 60; - a group write offering Minimal access says it is granted only on a top-level group with a Premium or Ultimate license, while the LDAP group link takes it on any group; - Planner and Security Manager carry no tier, since Community Edition grants both. Reading the routes at v19.5.0-ee found four more, corrected here too: a project access token takes Owner from a creator who holds it, which GitLab's create_service_spec asserts and the description denied; a project share takes Security Manager and Owner, which its hint left out; a custom role cannot extend Owner, since MemberRole validates the level against options_for_custom_roles; and a broadcast message cannot target Security Manager, which ALLOWED_TARGET_ACCESS_LEVELS leaves out. The curated gitlab_member_role and gitlab_project lines in tools_meta.json said the same and are corrected by hand. A project token's level is not simply capped at its creator's role either: Gitlab::Access.level_encompasses? lets only an Owner, or a member who holds the same role, assign Planner or Security Manager, so a Maintainer is refused both with a 400. The project token's description and its validation hint say so, which TestProjectCreate_PlannerFromAMaintainer_IsExplainedAsOwnersToGrant holds; a group token needs no such sentence, since only an Owner creates one. TestBuildActionCatalog_MembershipAccessLevels_OfferOnlyWhatTheRouteAccepts holds 32 action parameters to sets written as numbers in the test, independent of the table: the enum, every N=Role the description offers, the levels it leaves out, a tier on Planner or Security Manager, and Minimal access offered without its condition. A level parameter it does not map fails it, and so does a mapping nothing publishes. The R-ENUM exemptions for these fields cite the routes and models rather than the API pages, which listed levels the routes refuse, and name every value the schemas stopped offering. Two end-to-end probes run at the top of the stack: a project access token minted at Owner, and an Owner-based custom role that GitLab refuses. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…age profile cannot see Measuring the packages this branch touches found two places a coverage profile has no block for, neither written by it, so the mutation run reported each of them as not covered. groupmembers built its published action IDs by joining two constants, domainGroup and the spec's name, and a constant expression has no block in a coverage profile. The IDs are written out whole, and TestActionIDs_EachIsTheGroupDomainAndItsSpecName holds each to the spec it names, which is what the join used to guarantee; the catalog test beside it already holds every ID to the catalog the server builds. The granular scope namespace in accesstokens was a tagless switch, whose case expressions a coverage profile does not cover either. It is two ifs, with the same answers. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
The last case of the groupsaml options switch was only ever true, since every caller names one of the five tools, and gobco reported it as the one condition of the package measured one way. A test with a name none of the cases holds measures it both ways, and finds the generic metadata and no schema override. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
… how a level parameter takes its set Row 110 of docs/development/upstream-bugs.md said this server's own schemas offered levels GitLab refuses and that issue 1280 tracked it. Its summary cell and its Workaround bullet now say what the server does, and link the issue rather than name it bare: every access level parameter publishes only what its route accepts, from toolutil.AccessLevelSet, and both approvals refuse a level their scope does not grant before the request, with the group's Minimal access read back after it. Its Problem bullet says why such an approval reads like one, and what else the failed approval does on GitLab's side. Row 141 is new. Reading the routes found two pages the documentation merge request of row 110 does not touch: doc/api/member_roles.md lists Owner for a custom role's base level and doc/api/broadcast_messages.md lists Security Manager as a target, and GitLab refuses both. Neither is reported upstream yet, which its own Summary row now says, and row 110 points at it. The meta descriptions page's example of a value set the prose and the schema disagreed on keeps its history and says the schema offers 10 to 40 now. Its premise, that a numeric value set cannot be an enum, stopped holding when the access level parameters became integer enums: the page, the audit's package comment and three of its function comments now say what does hold, that the audit folds only an enum's string values, so a numeric set is judged against the property's own description. The create-mcp-tool skill says how a new access level parameter takes its set and which test holds it. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…dings without a comparison operator The comments the previous commit corrects make this package one the branch touches, and measuring it found two things no test could hold, both older than the branch. main called os.Exit directly, so its statements were the 1.8% the package's tests never ran. It exits through osExit, the variable the other commands' mains use, and TestMain_ExitsWithTheCodeRunReturns runs it with -check over the served surface and reads the code it exits with. The findings were ordered by a comparator of four < comparisons, each guarded by a != on the same part, and the mutation run left all four boundary mutants alive: two parts that differ order alike under < and <=, and two findings equal in every part cannot be told apart wherever they land, so no test can kill them. The key is compared with strings.Compare under cmp.Or, as the closed servers audit does, which orders exactly as before and leaves no operator to mutate; the four ordering tests are unchanged and pass. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
|
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 |
|---|---|
Shared access-level sets and schema enforcement internal/toolutil/*, internal/tools/action_specs.go, internal/tools/action_catalog_test.go, .github/skills/create-mcp-tool/SKILL.md |
Adds route-specific access-level sets, choice formatting, schema overrides, and canonical filter enums. Tests check the set definitions and published tool schemas. |
Approval validation and membership checks internal/tools/accessrequests/*, test/e2e/gitlab/{common,ce,ee}/accessrequests_test.go, test/e2e/gitlab/ee/actions_test.go |
Approval inputs distinguish omitted levels from explicit values. Approval handlers reject unsupported levels; group Minimal access approvals check eligibility and verify membership after approval. |
Membership, invitation, and sharing routes internal/tools/{groupmembers,groups,invites,members,projects}/* |
Route schemas and guidance use the relevant access-level sets. Error hints use the same choices, and group-share validation checks values against the shared set. |
Specialized access-level routes internal/tools/{accesstokens,adminspecs,broadcastmessages,groupldap,groupsaml,memberroles,dynamic}/*, test/e2e/gitlab/{common,ee}/* |
Access-token, broadcast-target, directory-link, and custom-role schemas and guidance reflect their accepted levels. Tests cover token creation and rejection of an unsupported custom-role base level. |
Audits and supporting documentation cmd/audit_1to1/internal/enums/exemptions.go, cmd/audit_meta_descriptions/*, docs/development/*, CLAUDE.md |
Updates audit exemptions and audit-meta-description guidance. Development and upstream-bug documentation records access-level behavior and test-file counts. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- jmrplens/gitlab-mcp-server issue 1530: The issue describes the same schema contract for route-accepted values. This pull request applies it to access-level parameters, while the issue’s merge-request parameters are outside this change.
- jmrplens/gitlab-mcp-server issue 1508: The issue concerns clients that reject non-string enum values. This pull request adds integer access-level enums but does not add Gemini-specific handling.
Suggested labels: bug, mcp, documentation
Suggested reviewers: sourcery-ai
Merge Risk: 
Minimal · up to b0517
-
The changes appear mergeable after normal checks; no concrete unresolved regression was established.
Pre-merge checks: 
5
-
Full details: Description check
-
Explanation
The description is detailed and covers the issue, implementation, tests, coverage, compatibility, and generated artifacts. It does not follow every template heading or checklist format, but it provides the required information in equivalent sections.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 94.20% 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 69 functions across 48 files. (6 skipped: 5 unsupported, 1 too large.)
-
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.
|
…ists, and publish the eight sort orders they document (#1573) Refs #1165 This is the bottom layer of the first #1165 stack. It classifies every action that publishes `page_token` by what GitLab 19.5.0 does with a keyset request on its route, and it changes the two service account token lists, the two routes where GitLab answers every keyset request with 405, which the issue's 2026-10-10 decision covers. On those two lists it removes the keyset input and an `order_by` GitLab never reads, and it publishes the eight sort orders the routes document in place of the `asc` and `desc` they could not use. ## The classification I read every route at `v19.5.0-ee` of GitLab's own source. Each action's routes come from `docs/development/action-requests.json`, each route was matched to its Grape `get`, and the endpoint body or the helper it calls was read for its paginator: offset `paginate` never reads `pagination`; `paginate_with_strategies` takes keyset with `CursorPager` (cursor `cursor`) when the relation's class has the requested ordering among its `supported_keyset_orderings`, falls back to `Gitlab::Pagination::Keyset` (Project ordered by `id` alone, `id_after`/`id_before`), and otherwise answers `405 Keyset pagination is not yet available for this type of request`; `GitalyKeysetPager` serves branches, tags, the tree and, behind a flag, commits by `page_token`. `page_token` is published by 202 individual tools, which carry 204 actions (`gitlab_commit_list` serves `repository.commit_list` and `repository.file_history`, and `gitlab_issue_list_group` serves `group.issues` and `issue.list_group`). | Class | Actions | What GitLab does with `pagination=keyset` | | --- | --- | --- | | follows keyset | 13 | pages by keyset for every ordering the action can send | | 405 by ordering | 5 | pages by keyset for some orderings and answers 405 for the rest | | 405 always | 2 | answers 405 whatever the request | | keyset not followed | 1 | answers 405 with the default ordering and, ordered by `id`, serves a keyset page but never applies the position its own next link names | | ignored | 183 | never reads it: 177 routes page by offset `paginate`, 6 are unpaginated or a single object | The 13 that follow keyset are the three audit event lists (`cursor`), `branch.list`, `tag.list` and `repository.tree` (`page_token`), `issue.list` (`cursor`), `package.registry_tag_list` (`last`), `runner.jobs`, `user.list`, `user.list_service_accounts` and the group and project service account lists (`cursor`). Two of them carry a condition: `branch.list` with `search` or `regex` uses the legacy finder, where keyset needs `branch_list_keyset_pagination` (off by default) and is ignored, and on `issue.list` an `order_by` outside the Issue keyset orderings makes GitLab delete `pagination` and answer offset. The 5 refused by ordering are `project.list`, `project.list_forks`, `project.list_user_projects` and `project.list_user_starred`, which follow keyset only with `order_by=id` (the default is `created_at`, so a keyset request that leaves `order_by` out is refused), and `job.list_project`, which follows keyset only ordered by `id`, its default. The 2 refused always are `group.service_account_pat_list` and `project.service_account_pat_list`: `paginate_with_strategies` over a PersonalAccessToken relation, a class with no keyset ordering that is not a Project. The one not followed is `project.list_user_contributed`. Its route reaches the same paginator as the other project lists, so a keyset request with the default `created_at` ordering is answered with 405 as theirs is. With `order_by=id` it is served a keyset page, but the route builds `ContributedProjectsFinder` with no params (`lib/api/projects.rb:396`), so the `id_after` or `id_before` its next link names is never applied and every keyset page is the first one again. That is the upstream bug register's row 112, and `project.list_user_contributed` already refuses `pagination` and `page_token` before any request, so no caller reaches either answer. By the issue's decision its keyset input should not be offered either. It shares `userProjectFilterInput` with the two user project lists that do follow keyset, so removing it means splitting that input in `internal/tools/projects/projects.go`. I left it to the layer of the second #1165 stack that removes the keyset input from every list that cannot use it: that layer changes the file anyway, and it holds this action to the same rule as the lists GitLab pages by offset. The table agrees with the 20 `toolutil.KeysetContinuation` call sites. The 18 actions that follow keyset or are refused by ordering pass it, in 17 sites: one, in `listUserScopedProjects`, serves both `project.list_user_projects` and `project.list_user_starred`. `project.list_user_contributed` calls the same helper but refuses keyset before reaching it, and the two refused always pass none. The other three sites sit on routes classed ignored. Two of those are dead: `group.ssh_cert_list` and `project.upload_list` page by offset `paginate`. The third is not: `repository.commit_list` and `repository.file_history` follow keyset by `page_token` when `commits_keyset_pagination` is on, a GitLab.com rollout flag that is off by default on 19.5.0, so that site has to stay when the offset lists lose their keyset input. Three routes behave differently from what I expected before reading them. Every `order_by` that `/users` declares is a User keyset ordering, so the users list refuses none. Project issues fall back to offset rather than answering 405. The audit event lists send no ordering, and their routes declare none. So the only ordering refusals a caller can reach are the four project lists above and project jobs. <details> <summary>The 21 actions GitLab can page by keyset, or refuses it on</summary> | Action | Route | GitLab `v19.5.0-ee` | Class | Cursor | | --- | --- | --- | --- | --- | | `audit_event.list_group` | `GET /groups/:id/audit_events` | `ee/lib/ee/api/groups.rb:257` | follows keyset | `cursor` | | `audit_event.list_instance` | `GET /audit_events` | `ee/lib/api/audit_events.rb:55` | follows keyset | `cursor` | | `audit_event.list_project` | `GET /projects/:id/audit_events` | `ee/lib/ee/api/projects.rb:63` | follows keyset | `cursor` | | `branch.list` | `GET /projects/:id/repository/branches` | `lib/api/branches.rb:83` | follows keyset | `page_token` | | `group.service_account_list` | `GET /groups/:id/service_accounts` | `lib/api/group_service_accounts.rb:105` | follows keyset | `cursor` | | `issue.list` | `GET /projects/:id/issues` | `lib/api/issues.rb:272` | follows keyset | `cursor` | | `package.registry_tag_list` | `GET /projects/:id/registry/repositories/:repository_id/tags` | `lib/api/project_container_repositories.rb:111` | follows keyset | `last` | | `project.service_account_list` | `GET /projects/:id/service_accounts` | `lib/api/project_service_accounts.rb:97` | follows keyset | `cursor` | | `repository.tree` | `GET /projects/:id/repository/tree` | `lib/api/repositories.rb:170` | follows keyset | `page_token` | | `runner.jobs` | `GET /runners/:id/jobs` | `lib/api/ci/runners.rb:352` | follows keyset | `cursor` | | `tag.list` | `GET /projects/:id/repository/tags` | `lib/api/tags.rb:62` | follows keyset | `page_token` | | `user.list` | `GET /users` | `lib/api/users.rb:267` | follows keyset | `cursor` | | `user.list_service_accounts` | `GET /service_accounts` | `lib/api/service_accounts.rb:77` | follows keyset | `cursor` | | `job.list_project` | `GET /projects/:id/jobs` | `lib/api/ci/jobs.rb:72` | 405 by ordering | `cursor` | | `project.list` | `GET /projects` | `lib/api/projects.rb:248` | 405 by ordering | `id_after` / `id_before` | | `project.list_forks` | `GET /projects/:id/forks` | `lib/api/projects.rb:248` | 405 by ordering | `id_after` / `id_before` | | `project.list_user_projects` | `GET /users/:user_id/projects` | `lib/api/projects.rb:248` | 405 by ordering | `id_after` / `id_before` | | `project.list_user_starred` | `GET /users/:user_id/starred_projects` | `lib/api/projects.rb:248` | 405 by ordering | `id_after` / `id_before` | | `group.service_account_pat_list` | `GET /groups/:id/service_accounts/:user_id/personal_access_tokens` | `lib/api/group_service_accounts.rb:240` | 405 always | none | | `project.service_account_pat_list` | `GET /projects/:id/service_accounts/:user_id/personal_access_tokens` | `lib/api/project_service_accounts.rb:217` | 405 always | none | | `project.list_user_contributed` | `GET /users/:user_id/contributed_projects` | `lib/api/projects.rb:396` | keyset not followed | `id_after` / `id_before`, never applied | </details> The other 183 actions, the ones GitLab pages by offset or not at all, are listed with their routes in the pull request that removes their keyset input, since that is where the list is acted on. `mr_review.changes_get` is classified like the rest (ignored: `lib/api/merge_requests.rb:719` pages the diffs by offset) and its code is left alone here: #1517 changes that handler and pages its diffs by offset, and the unused `order_by`, `sort`, `pagination` and `page_token` on its input are #1530. ## What changed `ListPATInput` in `internal/tools/groupserviceaccounts` and `internal/tools/projectserviceaccounts` now embeds `PaginationInput` alone, has no `order_by`, and `ListPATs` passes a zero `KeysetPaginationInput` to `ApplyListOptions`, as the offset-only lists do. On these routes `pagination=keyset` is answered with 405, `page_token` without it is ignored by the offset paginator, which reads `page` and `per_page` alone, and `order_by` is declared by neither route, so the token finder, which is handed the declared params alone and orders by `sort`, never saw it. A caller that sends any of the three is now told the field is unknown before any request is made, and the published schema of both actions offers `page` and `per_page` and none of them. R-INPUT now sees three fields of client-go's `ListOptions` missing from each input, so I answered them in `acceptedMissingInputs` with one reason naming the two routes, the paginator, the 405 and the undeclared `order_by`. Both lists also published `sort` with the `asc` and `desc` enum the catalog gives every sort parameter. These routes take a named order, and document eight: `created_asc`, `created_desc`, `expires_asc`, `expires_desc`, `last_used_asc`, `last_used_desc`, `name_asc` and `name_desc` (`access_token_params` in `lib/api/helpers/personal_access_tokens_helpers.rb`). `PersonalAccessTokensFinder` sorts by a key of `PersonalAccessToken.simple_sorts` and leaves the tokens unsorted for any other value, and `asc` and `desc` are not keys, so the schema refused every order GitLab applies and admitted only two values it ignores. The `pat_list` action of each package now overrides the enum with the eight documented orders, as the three access token lists already do, and the description names them. That is its own commit, since it is not about keyset. `simple_sorts` holds nine more keys, which `PersonalAccessToken` inherits from `Sortable` and the routes do not document: `id_asc`, `id_desc`, `created_at_asc`, `created_at_desc`, `created_date`, `updated_asc`, `updated_desc`, `updated_at_asc` and `updated_at_desc`. GitLab applies them, and the example request on both documentation pages sends `sort=id_desc`, a value outside the list printed above it. I kept the enum to the documented eight, as the access token lists do, said so in the comment beside it, and recorded the mismatch in the upstream bug register as row 143, a documentation fix to propose upstream. If we would rather publish what GitLab applies, the enum can take all seventeen; R-ENUM cannot tell the two apart, since client-go models the field as a `*string`. ## How I tested it I wrote and measured this layer on `e3e09d290` and then rebased the stack twice: onto `main` at `6c9066aef`, after the OpenSSF Best Practices stack, and onto `main` at `56758fb71`, after the access level and edition tier stack (#1567 to #1571). The rebases changed one thing in this layer, the number of its row in the upstream bug register: `main` had taken row 140 for gobco, and then row 141 for the custom role and broadcast message pages and row 142 for the API pages' tier markers, so the token list row is 143, in the register and in the comment beside each enum. In the Go files this layer touches, `main` changed the copyright and SPDX header at their top and, in `cmd/audit_1to1/internal/structs/analyze.go`, added three R-INPUT answers of its own (the notification `new_epic` and the two project checksum filters) away from the six this layer adds, which merged without a conflict. What I ran again after the rebases is at the end of this section. Tests first. On `e3e09d290`, `TestListPATs_KeysetArgument_IsRefusedBeforeAnyRequest` and `TestListPATInput_Schema_OffersOffsetPaginationOnly`, in each package, failed for every one of the three fields: the route sent the request to a mock that forbids any (`mock API was called 1 time(s); the test forbids any request`) and the schema offered the field (`schema offers order_by = true, want false`). `TestActionSpecs_TokenSortEnum_NamesTheEightDocumentedOrders`, in each package, failed with `pat_list sort enum = [asc desc], want [created_asc created_desc expires_asc expires_desc last_used_asc last_used_desc name_asc name_desc]`. All of them pass on this head. The existing keyset case of the group token list test now checks offset pagination instead, the project one no longer sends the keyset fields or `order_by`, and the group one sends `created_desc` rather than `desc`. On the layer's head before the rebases, with `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred` since it is below the stack's top: - `go test ./... -count=1`: all 294 packages pass. - `groupserviceaccounts`, `projectserviceaccounts` and `cmd/audit_1to1/internal/structs`: 100% statement coverage; condition coverage 158/158, 210/210 and 426/426; mutants Killed 75, 92 and 198, with Lived 0, Not covered 0 and Timed out 0 in all three. - `go run ./cmd/audit_1to1/ -gaps-only`: the backlog is byte for byte the one on `e3e09d290` (20 missing inputs, 0 stale declarations), so the six new answers are each used and no finding is new. `make audit-1to1` passes. - `golangci-lint run --build-tags e2e` and `golangci-lint fmt --diff` on the three packages, `go test -tags e2e -c ./test/e2e/gitlab/...`, and `make check-e2e-static`, `check-test-file-names`, `check-test-subtests`, `check-test-goroutines`, `check-action-ids`, `check-gateway-chars`, `check-dead-consts`, `check-meta-descriptions`, `check-surface-quality`, `check-tenancy`, `check-doc-tool-names`, `check-md-tables`, `check-md-escaping`, `check-sdk-context` and `audit-catalog-first`: all pass. - `docs/development/upstream-bugs.md`, for the token list row: markdownlint and `node scripts/check-doc-links.mjs` pass. With the comparison checked, `TestToolSnapshots_Individual` and the compact and full cases of `TestToolSnapshots_MetaParamSchemaModes` report exactly the two token list tools and the `gitlab_group` and `gitlab_project` schemas that carry them, which is the regeneration the top does. The curated `tools_meta.json` does not change. No catalog action is added or removed, and the e2e scenarios of both token lists send neither keyset field, `order_by` nor `sort`, so no scenario changes and no Docker run was made for this layer. After the second rebase, on this head: `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred go test ./... -count=1` passes (301 packages), `golangci-lint run --build-tags e2e` and `golangci-lint fmt --diff` pass on the three packages, the e2e packages compile, and every gate of the list above passes again, `make audit-1to1` included. `go run ./cmd/audit_1to1/ -gaps-only` writes a backlog byte for byte the one `main` writes at `56758fb71`, which is the one it wrote at `6c9066aef` (20 missing inputs, 0 stale declarations). `make check-file-headers` and `make check-dco` against `main` pass. Markdownlint and `node scripts/check-doc-links.mjs` pass at the top of the stack, which holds the register as it is here. I did not measure coverage, conditions or mutants again: the rebases changed no statement this layer wrote in the three packages, only the comment that names the register row and the header lines at their top. ## What is left for the top of the stack This layer regenerates no committed artifact, under the freshness deferral. The top regenerates the tool snapshots, the per-domain tool reference in both languages, the token footprint, the llms files, the request inventory and the testing reference once. #1517 regenerates some of the same files (the tool snapshots, the request inventory, the token footprint, the testing reference and the llms files), so whichever of the two lands second regenerates them again. The two share no hand-written file. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…en to them in R-PAGE (#1574) 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 by `cmd/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's `page_token` to the routes the action sends, and every list GitLab can page by keyset to the `next_page_token` it is followed by. Nothing in the record could give that answer before, because GitLab reads `pagination=keyset` without declaring it: of the 53 routes whose code reaches a keyset paginator at `v19.5.0-ee`, four declare `pagination` and 22 declare no cursor either. ## What the record says now Each route whose code reaches a keyset paginator carries a `keyset` list, 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 without `keyset` reaches no keyset paginator, so GitLab answers it with an offset page whatever `pagination` says; 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_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, not the relation a request builds. So a `paginate_with_strategies` site carries `CursorPager`'s `cursor` unless the call passes `use_cursor: false`, which assumes a model with keyset orderings. The two service account token lists page `PersonalAccessToken`, which declares none: the record gives them `cursor`, and GitLab answers every keyset request there with 405 and never reads it. The comments of the field, of `apilive.KeysetSite` and 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 with `gitaly_pagination: true` (the internal Orbit commit list), a route writing its own next link or cursor headers through `Gitlab::Pagination::Keyset::HeaderBuilder` (the instance audit events, the registry tags, the work item discussions), and `keyset_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 has `CursorPager`, `Keyset::Pager` and `GitalyKeysetPager` each write a next link through the method GitLab calls after serving a page, and reads the parameter back (`cursor`, `id_after` and `id_before`, `page_token`, and `cursor` for the `X-NEXT-CURSOR` header). 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.rb` takes 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, following `super` into the CE method an EE module overrides. It records a site at a call to one of the paginators above; the cursor of `paginate_with_strategies` is `CursorPager`'s unless the call passes `use_cursor: false`, the cursor of a route's own next link is the key of the hash it hands the builder, and the cursor of `keyset_paginate` is the param it is handed. A feature flag is recorded on the sites it guards when it is tested as `if Feature.enabled?(:flag)` or `unless Feature.disabled?(:flag)`, which reads unambiguously from the branch instructions; that is how the commit list comes out behind `commits_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 without `keyset_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.0` alone on the remote Docker host, the digest the record already names (`sha256:f04c71f565601283971d6472dd466609f4679450526f39aab3e3dcd6d23dc0ec`), with `go 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 under `jq -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, `last` on the registry tags and `id_after`/`id_before` on the project lists included, except the two service account token lists, which the classification found refusing every keyset request and the record gives `CursorPager`'s `cursor`, 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`) reaches `GitalyKeysetPager` behind `commits_keyset_pagination`, which the record says is off by default, so it agrees with the classification on a default instance. `GET /groups` reaches `paginate_with_strategies` only for an anonymous caller (`present_groups_with_pagination_strategies` returns 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.keyset` in the report, `cmd/audit_1to1/internal/paths/pagination_keyset.go`) joins each action to the REST routes it sends through `docs/development/action-requests.json`, so it judges at the action's own grain, and reports four problems: - `page-token-on-offset-route`: the action offers `page_token` and 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 offer `page_token`. - `keyset-route-without-next-page-token`: the action offers `page_token` on a keyset route and its output carries no `next_page_token`. - `keyset-route-never-followed`: the action offers `page_token` on 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's `grain` says. 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_token` is 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` (`declaredKeysetRoutes` in `pagination_declarations.go`), each entry with what GitLab's handler does at `v19.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_tokens` and `GET /projects/:id/service_accounts/:user_id/personal_access_tokens`: `PersonalAccessToken` has 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_token` on one is reported, with the route in its `not_followable`, and a list sending one without `page_token` is 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_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`. 20 of them offer it on a keyset route, and every one publishes `next_page_token` and 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 without `page_token`, `group.search` and the two token lists, are declared. Taking `page_token` out of those 182 inputs is the next stack of #1165. ## Also in this layer A second commit leaves `cmd/gen_api_live` with no mutant the mutation run cannot judge. `runnerDetail` cut 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 with `max` and 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: onto `main` at `6c9066aef`, after the OpenSSF Best Practices stack, and onto `main` at `56758fb71`, 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, `main` changed only the copyright and SPDX header at their top; what it changed in the `Makefile`, `docs/development/cmd-utilities.md` and `docs/development/request-paths.md` lies outside the lines this layer edits there, and merged without a conflict. The six files this layer adds (`keyset.go` and its test in `cmd/gen_api_live` and in `cmd/internal/apilive`, `pagination_keyset.go` and its test in `cmd/audit_1to1/internal/paths`) now open with that header too, written by `make fix-file-headers` into 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, `a35673d89` before the rebases, compiled against it unchanged: `TestRunGenerate_ARouteCarryingAKeysetPaginator_ComesOutMarked` (the route came out with `keyset = <nil>`, and so did the probe and the count), `TestRunCheck_ARouteDeclaringACursorTheRecordDoesNotFollow_IsRefused` and `TestRunCheck_AVersion4RecordLabeled5_IsRefused` (the parent refused both records only as `schema 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 in `TestKeysetProblems_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 of `TestRunGenerate_AKeysetSiteNothingReads_IsRefusedAfterTheFineGrainedHalf` fails. 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, `1ece15937` before 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_PassesItsOwnGate` now also holds the summary's keyset figures to the findings they count. `cmd/audit_1to1/internal/paths`, `cmd/gen_api_live`, `cmd/internal/apilive` and `cmd/gen_action_grants/internal/join` are at 100% statement coverage, gobco reads 1114/1114, 254/254, 186/186 and 304/304 conditions, and `make coverage-mutants` reads 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/paths` also reports 483 mutants not covered, and every one is an `ARITHMETIC_BASE` on a `+` between string literals in a package-level initializer, on a line this layer does not change: the declaration tables of `sent_declarations.go`, `shape_declarations.go`, `endpoint_declarations.go`, `orbit_declarations.go` and `pagination_declarations.go`, a constant and a map of `published.go`, and a regular expression of `endpoints.go`. A `-` there does not compile, which is the tool artifact `docs/development/testing/README.md` describes; 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 analyze` passed 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-names` and `go run ./cmd/format_md_tables/ --check` passed on the head, `node scripts/check-doc-links.mjs` passed, `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred go test ./... -count=1` passed 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=1` passes (301 packages), `golangci-lint run --build-tags e2e` and `golangci-lint fmt --diff` pass on the four packages, the e2e packages compile, and `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-names` and `make check-md-tables` pass. R-PAGE's figures are the ones above: 53 keyset routes, 202 actions offering `page_token`, 20 of them on keyset routes, 182 on offset routes and 3 lists declared. `make check-file-headers` and `make check-dco` against 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.rb` in this pull request, and a fourth commit answers them without changing what the script records: - `rubydre:S7815`, a `raise` of a bare string in `next_link_of`: it now raises `MissingNextLinkError`, a `StandardError` of its own, with the same message. - `rubydre:S3776`, `KeysetWalk#scan` at a cognitive complexity of 25 against 15: `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`), and what it carried in locals from one instruction to the next is a `ScanState`. - `rubydre:S107`, `record_site` with ten parameters against seven: it takes the `ScanState`, 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?` and `unless Feature.enabled?`), nested blocks, helpers, a call to `super` and a recursive helper. Over five entry points both versions record the same 35 sites in the same order, with the same `via`, cursors and flags, and the probe fails with the same message. `go test ./cmd/gen_api_live/ ./cmd/internal/apilive/` and `make check-api-live` pass, 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`, the `case` in `scan` with 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_live` section and the R-PAGE paragraph of `docs/development/cmd-utilities.md`, the `check-api-live` row of `docs/development/static-analysis.md`, the R-PAGE paragraph of `docs/development/request-paths.md` and its line in `CLAUDE.md`) and `docs/development/testing/testing.md` are refreshed at the top of this stack, with the rest of its regeneration. This layer regenerates no committed artifact, so `go run ./cmd/gen_testing_docs/ --check` fails here on the new tests, as the `FRESHNESS` deferral 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_get` among the 182 actions offering `page_token` on 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. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
…lists end to end, and regenerate the stack's artifacts (#1575) 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`, the `gen_api_live` section 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 `-check` refuses of that half. The R-PAGE paragraph reads `keyset_routes` off the record's `keyset` field (53 routes, 39 of them offset too) rather than off the params (the 10 that declare `per_page` without `page`), and a new paragraph describes 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 that publish `next_page_token` and were seen following it, 182 on routes GitLab pages by offset alone, three lists declared). `docs/development/request-paths.md` says the same in its R-PAGE bullet, `static-analysis.md` and the repository map in a line each, and the R-PAGE clause of `CLAUDE.md` names 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 `pagination` or `page_token`, refuse either before sending anything and page with `page`, 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_PageByOffsetAlone` and `TestKeysetPagination_ProjectServiceAccountTokens_PageByOffsetAlone`, give a service account three tokens named so they sort in the order they were made, and on every surface: - send `pagination: "keyset"`, `page_token` and `order_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's `Unknown params` on the dynamic surface, and the meta dispatcher's `unknown field`; - follow the list one token a page by page number in `name_asc` order, holding every page to carry no `next_page_token` and to name the next page while `has_more` holds; - read it whole in `name_desc` order, 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 onto `main`, against CE 19.5.0, every surface failed at the first argument (8 tests, 8 failures), because that server still sends `pagination=keyset` and 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_params` on the individual surface and `the 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, with `Unknown 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 answers `405 {error: Keyset pagination is not yet available for this type of request}` for both `group.service_account_pat_list` and `project.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: - the tool snapshots of the individual surface and of the compact and full meta schemas, which change on exactly the two token list tools and the `gitlab_group` and `gitlab_project` schemas that carry them (`pagination`, `page_token` and `order_by` gone, `sort` naming the eight documented orders); the curated `tools_meta.json` does not change; - the per-domain tool reference of the group and project groups, in English and Spanish, the three llms files, the token footprint with the README block it writes, and the testing reference, refreshed with `-skip-coverage`; - the request inventory, 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 now 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. ## How I tested it The stack was written on `e3e09d290` and rebased twice: onto `main` at `6c9066aef`, after the OpenSSF Best Practices stack, and onto `main` at `56758fb71`, 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: `main` added rows to the pipe table of `docs/development/static-analysis.md` and padded it again, so I took `main`'s table, put this layer's sentence back into the `check-api-live` row and ran `go run ./cmd/format_md_tables/`. The second changed nothing in them: what `main` changed in `CLAUDE.md`, `docs/development/cmd-utilities.md` and `docs/development/repository-map.md` lies 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 from `main`'s new figures: 225 fewer tokens on each tier's individual surface, and 37 more test functions and three more test files under `cmd/`, the stack's own tests. With the freshness checks on, on this head after the second rebase: `go test ./... -count=1` passes (301 packages), and again with `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred`. `make analyze` passes 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 them `check-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-tenancy` and `check-file-headers`, with `check-openplugin` and `check-server-json-packages` run locally since they need the network. `make check-dco` passes for the ten commits of the stack (the tenth is the second layer's answer to SonarCloud's `rubydre:S131`, which changes no recorded output). `make test-e2e-http` (780 tests, 8 skipped because the runner has no Docker) and `make test-e2e-stdio` (277 tests) pass, the e2e packages compile, and `test/e2e/gitlab/common` passes `golangci-lint run` and `golangci-lint fmt --diff`. The site passes `i18n:check`, `facts:check`, `chips:check`, `llms:check`, `build` and `lint`, and `node scripts/check-doc-links.mjs` passes. Against Docker GitLab 19.5.0, the complete CE run (`make test-e2e-ce` with 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 `.env` with 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_live` runs, inside a GitLab it boots. What `main` brought that those suites exercise is the dependency update of #1564 (`golang.org/x/net` 0.62.0, `google.golang.org/grpc` 1.85.0, `gqlparser` 2.5.64 and `tiktoken-go` 0.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 and `new_epic` from a project's notification settings, and scenarios of its own in `accessrequests_test.go` and `accesstokens_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 classifies `mr_review.changes_get` with the rest, among the 182 actions offering `page_token` on an offset route, no declaration of it names that action, and nothing here changes it. Its unused `order_by`, `sort`, `pagination` and `page_token` are #1530 and the pre-check's `position_type` is #1531, both after #1517. The two share no hand-written file: this stack changes `cmd/audit_1to1/internal/structs/analyze.go`, the `groupserviceaccounts` and `projectserviceaccounts` packages, R-PAGE in `cmd/audit_1to1/internal/paths` (`analyze.go`, `pagination*.go`, `shapes.go`) and the live record's generator, and #1517 changes `sent_declarations.go` in the same package, the `mr*` packages, `internal/toolutil/diff_position.go` and `internal/tenancy`, so a rebase of either is mechanical. Both change generated files: `internal/tools/testdata/tools_individual.json`, `tools_meta_compact.json` and `tools_meta_full.json`, `docs/development/request-inventory.json`, `docs/development/token-footprint.md` and `site/src/data/token-footprint.json` with the README token block, `docs/development/testing/testing.md`, `llms-full.txt` and `llms-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.txt` and the `mr-review` tool reference). --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
… copy between two to every field they share (#1635) Refs #1595 Refs #1600 This is the first layer of the auditor stack that closes the 1:1 audit's blind spots before client-go is bumped. Nothing asked whether every client-go struct a request sends is compared with an input, and several shapes escaped the field diff: a struct a helper builds out of scalars or out of a struct of arguments (`search.searchOpts`, `labeldata.NewProjectListOptions`, `runners.runnerListRequest`) was compared with nothing a model sends; a nested struct such as `BranchPermissionOptions` was compared with the handler's input and dropped as disjoint; a struct a raw request takes as `any` was not in view at all; and the group hook edit is a field-by-field copy of the add options, so a field both structs gain is silently left out. That last one is what v3.38.0 does with `URLVariables`. ## What the check does The pairing behind R-INPUT now follows a call. A struct a helper builds is paired with the input of the handler that hands the helper its fields, across a tool package boundary and through a struct of arguments such as `search.scopedSearchArgs`. A function that takes an input and references no service method (`projects.buildListOpts`) is a helper too. An intermediate the handler fills from its input carries it into a closure's parameter (the runner lists), into a helper named as a value (the issue statistics) and into a helper that sends the options itself (the access token creates). A struct of ours that embeds a client-go request (`groupboards.updateGroupBoardBody`) builds it. A nested struct is paired with the input element its values are read from, through the locals they are assigned to. Each such pair says how it was found (`via: caller` or `via: nested`). The sdk scope gains an `option_structs` section, gated in every `go test ./...` like the rest of the scope. Rule B holds every struct a referenced service method takes, and every struct a raw request built through client-go's constructors carries (typed, embedded in a struct of ours, or handed one call up as `any`, which is how `projects.doProjectRequest[T]`, `mrapprovals.rawMutateApprovalRule` and the `groupboards.newRawRequest` seam take theirs), to an input a model sends: paired, converted from a held struct, copied from one, or declared. A struct compared with an unexported intermediate alone holds nothing. Rule A holds a copy to every exported field the two structs share under one name and type. Rule C holds every nested struct of parameters a held request reaches to the input element it is built from, and gates on a field that element lacks. A struct that carries an `io.Reader` (the four avatars) is upload content and not a struct of parameters. `optionStructDeclarations` answers what nothing fills, with `map-input`, `sdk-models-unused` (which requires every call to send nil), `constant-lookup`, `scalar-list` (which requires the struct to keep one field) and `dropped-on-purpose` for a field a copy leaves out. A declaration that answers nothing, sits on the wrong kind of key or no longer holds what its category requires fails the gate. ## Witness tests, red before and green here The witnesses on the tree, `TestCollectPairs_Repository_PairsWhatAHelperBuildsWithTheInputOfItsCaller`, `TestCollectPairs_Repository_PairsANestedStructWithTheInputItsElementsComeFrom`, `TestRun_Repository_HoldsEveryOptionStructAHandlerSends` and `TestAcceptedEnumGaps_NotificationLevels_ExcuseEachValueAndNotAConstantClientGoAdds`, were copied unchanged into a checkout of main as it stood before the Orbit migration (9146b5f) and run there with `go test ./cmd/audit_1to1/internal/structs/ ./cmd/audit_1to1/internal/sdk/ ./cmd/audit_1to1/internal/enums/ -count=1 -v -run '<the witnesses>'`: 55 failures and no pass, for example `CollectPairs lists no input pair "runners ListGroupInput ListGroupsRunnersOptions"`, `CollectPairs lists no input pair "projectimportexport ScheduleExportUploadInput ScheduleExportUploadOptions"` and, for the notification level, `missing = [0 1 2 3 4 5 6], extra = [custom disabled mention participating watch]; want the new constant alone missing and no name extra`. On this branch the same command gives 55 passes and no failure. The stand-in trees cover each shape on its own: two equal hook structs with a field-by-field copy are green, a field both gain turns the copy red with that field, and a conversion turns it green again; a nested permission struct that gains `member_role_id` is a missing input on the input element; a helper handed constants, or an intermediate filled with them, leaves its struct unpaired; a closure's parameter, a helper named as a value, a builder that takes an input and a helper that sends the options itself each pair with the caller's input; a raw request's `any` is followed one call up, through a function, a generic one and a package variable, and a struct embedded in one of ours is received, while a variadic `any` is not; a local is followed to what it is assigned, a local that refers to itself once; a handler whose input has no name pairs nothing; and every way a declaration goes stale is listed. As evidence for the bump stack, I ran the gate over a scratch copy of this tree with client-go moved to v3.38.0 in go.mod and go.sum and nothing else changed. It fails on what the bump must fix: the group hook copy shares 31 fields and copies 30, missing `URLVariables`, and the new nested `HookURLVariable`, which all four hook options structs now reach, is unpaired (besides `GroupPlaceholderReassignments`, the new service R-ACTION already reports). ## What it found on main, and how each is answered Of the 566 structs the section holds, 560 are paired with an input a model sends, among them the six that were paired with an unexported intermediate alone (the two token creates, the two issue statistics and the two scoped runner lists) and five of the six that only a raw request taking `any` sends (the project hook list and add, the two merge request approval rule structs and the group board update; the sixth, the project hook edit, is one of the conversions below). `CreateSecurityAttributesOptions` is not in the section, since no call sends it: the handler sends its own GraphQL mutation. `CreateProjectForUserOptions`, `EditHookOptions` and `EditProjectHookOptions` are conversions. The group hook edit is paired with the edit input that reaches it through the one copy, which is complete (30 of 30). Three structs are declared: `UpdateSettingsOptions` (`map-input`), `ListActiveIntegrationsOptions` (`sdk-models-unused`, sent as nil) and `GroupAccessLevel` (`scalar-list`, since the input offers `allowed_to_push` and `allowed_to_merge` as `[]int`). The nested pairs lack four fields, answered in `acceptedMissingInputs`: `access_level_description` on the four protected environment approval rule inputs, which GitLab does not declare. `ScheduleExport` read its upload destination off the nested input or the deprecated flat fields into two locals; it now reads the flat fields as the nested input they stand for, so `ScheduleExportUploadInput` is the one compared with `gl.ScheduleExportUploadOptions`, and the request is unchanged. The new pairs take R-INPUT from 562 to 664 input pairs. I declared 57 omissions where the route does not declare the param (the audit event lists and project iterations order nothing; every search pages by offset; `ref` reaches no repository in the issue, merge request, milestone, note, project, snippet and user searches). The three params GitLab declares and client-go models that the new pairs found missing are offered here: `archived` on `labels.ListInput`, and `ref` on `search.CommitsInput` and `search.WikiInput`, each with a test that the query carries it. The new pairs also put 31 more enum fields under R-ENUM (621 to 652). They found the token creates describing their roles wrongly, which #1567 had already corrected on main. Every gap is declared value by value: 0, 5 and 60 on the two token creates, 0, 5, 50 and 60 as the group member role's base level, `min_access_level` 0, 5 and 60 on the five project lists, and the notification level, an int whose JSON is its name, declared per constant and per offered name so that a constant client-go adds is a finding. The tool reference, the tool snapshots and the other committed artifacts the new parameters and descriptions move are regenerated once at the top of the stack. ## Checks 100% statement coverage and full condition coverage (gobco) on every touched package: `cmd/audit_1to1/internal/structs` (702/702), `cmd/audit_1to1/internal/sdk` (316/316), `cmd/audit_1to1/internal/enums` (154/154), `internal/tools/projectimportexport` (112/112) and `internal/tools/search` (296/296); gobco cannot instrument `internal/tools/labels`, whose external tests read `export_test.go`, on main as here. `make coverage-mutants` leaves no mutant lived on any of them: structs 266 killed, sdk 166, enums 99, search 219, projectimportexport 56 and labels 49. Two structs mutants time out, the closure's growth test and `appendNamed`'s identity test, which turn the fixed point into a loop that never ends. The not covered mutants left are the ones main has in files this layer does not touch: the string concatenations of `action_specs.go` constants in labels (8) and projectimportexport (5); enums has none. `go test ./... -count=1` with `GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred`, golangci-lint, the dead constants, test subtests, test goroutines, test file names, closed servers, Markdown escaping, action IDs, meta descriptions, catalog-first, e2e static, surface quality, SDK context, gateway characters, doc tool names, the fine-grained derivation, the Go doc audit, `-scope=sdk`, `-scope=enums`, `-scope=structs`, `-scope=paths`, `-scope=required` and `-scope=grants`, markdownlint and the doc links all pass. Each of the three commits builds, vets and passes the auditor's suites and the tests of the three tool packages on its own. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>



Closes #1280
Our access level schemas offered values GitLab refuses, and the two access request approvals answered two calls wrongly.
access.approve_projectandaccess.approve_groupoffered0=No accessand then sent no level for it, so GitLab applied its default and the requester became a Developer. Given a level GitLab does not grant there (5on a project, or on a group that is not a top-level group of a licensed instance), GitLab answers the approval with201and the requester while the request stays pending, and both actions reported that as an approval. I read the approval service at v19.5.0-ee to be sure: it sets the level on the request, the save fails the member's validation, and the route presents the unsaved record, so nothing in the answer tells the two apart.I put the levels each family of routes accepts in one table,
toolutil.AccessLevelSetininternal/toolutil/access_level.go, read from the v19.5.0-ee source: the filters and every project write take Guest to Owner, the group writes take Minimal access as well (only on a top-level group with a Premium or Ultimate license), an LDAP group link takes Minimal access on any group, access tokens take Guest to Owner, a broadcast message cannot target Security Manager, and a custom role extends Guest to Maintainer.Every membership access level parameter now publishes its set as an integer enum and as an
N=Rolelist in its description.min_access_levelandshared_min_access_levelget theirs centrally, since every route that takes them declares the samevalues:, and the writes through their ActionSpecs. The handlers' hints and the parameter guidance read the same table. Planner and Security Manager lost their "(Premium/Ultimate)" label, because Community Edition grants both.The approvals take
access_levelas optional (*int). A level the scope does not grant,0included, is refused before any request, and leaving it out sends none, so GitLab grants Developer. Minimal access is the one level a group grants in some places and not in others, and the approval's answer is the same either way, so a group approval at Minimal access is refused without a request on the Free tier, reads the group and is refused on a subgroup before the approval, and otherwise reads the requester back among the group's members after it, answering a requester who is not there as a request still pending. Both reads are marked as optional requests for the fine-grained derivation, and the member read goes out under the caller's context rather than the approval's capture, so the membership reported is the approval's own answer. Each handler still buildsApproveAccessRequestOptionsitself from its own input, so the 1:1 audit keeps pairing the two inputs with client-go's options and R-ENUM judges the levels they offer.Reading the routes for this found four more corrections beyond the issue's list, which I made too: a project access token takes Owner from a creator who holds it (GitLab's own
create_service_specasserts it, and the description said it was invalid), while Planner and Security Manager are granted only by an Owner of the project or by a member who holds that role (Gitlab::Access.level_encompasses?keeps both outside the hierarchy, so a Maintainer is refused them with a 400, and the token's description and hint now say so), a project share takes Security Manager and Owner (the hint said10/20/30/40only), a custom role cannot extend Owner (MemberRolevalidates againstoptions_for_custom_roles, thoughdoc/api/member_roles.mdlists it), and a broadcast message cannot target Security Manager (ALLOWED_TARGET_ACCESS_LEVELShas none, thoughdoc/api/broadcast_messages.mdlists it). The curatedgitlab_member_roleandgitlab_projectlines intools_meta.jsonsaid the same and are corrected by hand. The R-ENUM exemptions for these fields now cite the routes and models rather than the API pages, and name every value the schemas stopped offering. Register row 110 now says what the server does, and a new row 141 records the two upstream pages, which nobody has reported yet. The meta descriptions audit's premise that a numeric value set cannot be an enum stopped holding once these parameters became integer enums, so its page and comments now say what does hold: it folds only an enum's string values and judges a numeric set against the property's own description.Measuring the touched packages found what no test reached, none of it written by this branch, which I fixed in three commits of their own:
groupmembersbuilt its published action IDs by joining two constants, an expression no coverage profile reaches, so they are written out whole and a test holds each to the spec it names; the token scope switch inaccesstokensis two ifs, for the same reason; the last case of thegroupsamloptions switch is now measured both ways; andcmd/audit_meta_descriptions, touched for its comments, exits through theosExitseam the other commands use, so itsmainis measured, and orders its findings withstrings.Compareundercmp.Or, which orders exactly as before and leaves nothing for the four boundary mutants the old<comparator could never kill. Thecreate-mcp-toolskill says how a new access level parameter takes its set.I wrote the tests first and ran them against the parent.
TestApprove_LevelGitLabWouldNotGrant_IsRefusedBeforeTheApproval(seven cases: 0, 5 and 60 on a project, 0 and 60 on a group, 5 on the Free tier and on a subgroup),TestApproveGroup_MinimalAccessLeftPending_IsNotReportedApprovedandTestApproveGroup_MinimalAccessOnALicensedTopLevelGroup_IsReadBackAndApprovedall failed there: every approval was answered as granted, and no group or member read was sent. The catalog-wideTestBuildActionCatalog_MembershipAccessLevels_OfferOnlyWhatTheRouteAcceptsholds 32 action parameters to sets written as numbers in the test, independent of the table, and failed on every one of them.TestProjectCreate_PlannerFromAMaintainer_IsExplainedAsOwnersToGrantfailed on the description and hint that said only "no higher than the caller's own". All of them pass on this branch.Every touched package is at 100% of statements. gobco reports every condition both ways: toolutil 4380/4380, internal/tools 406/406, dynamic 1910/1910, projects 1390/1390, groups 970/970, accesstokens 320/320, enums 156/156, the meta descriptions audit 128/128, memberroles 126/126, members 104/104, groupsaml 82/82, groupldap 72/72, broadcastmessages 66/66, invites 60/60 and adminspecs 40/40, and accessrequests 128/128 and groupmembers 180/180 measured in a scratch copy without their external catalog test, which gobco's bridge cannot follow, so those two are lower bounds. gremlins leaves no mutant lived, not covered or timed out: on whole packages, 53 killed in accessrequests, 84 in groupmembers, 37 in groupsaml, 8 in adminspecs, 134 in accesstokens, 55 in the meta descriptions audit, 65 in memberroles, 59 in members, 38 in broadcastmessages, 34 in groupldap and 33 in invites; on the changed files of the larger packages, 74 in toolutil's
access_level.goandaction_spec.go, 491 in projects'projects.goandaction_specs.go, 219 in groups'groups.go,sharing.goandaction_specs.go, 15 in dynamic'sexample.goand 4 in the enum audit'sexemptions.go. Each of the seven commits builds and passesgo test ./... -count=1on its own withGITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred, and R-ENUM is clean at each. On the head golangci-lint (with thee2etag) passes on the touched packages, the three end-to-end packages compile, and these gates pass: dead constants, test goroutines, case loops, Markdown escaping, action IDs, R-PATH, meta descriptions, catalog-first, e2e static, surface quality, SDK context, tenancy, the fine-grained derivation, R-GRANT, R-INPUT, R-ENUM, closed test servers, godoc, gateway characters, documentation tool names, test file names, Markdown tables, markdownlint and the documentation links.make check-action-grantsandgen_testing_docs --checkreport the fine-grained permissions pages and the testing reference stale, which the top of the stack regenerates.R-PATH passes here against the committed request inventory, which does not hold the approval's two new reads yet. To see what the top's regeneration makes of them, I recorded the whole suite into a scratch inventory: it differs from the committed one by exactly those two rows (
GET /groups/:group_idwithwith_projects, andGET /groups/:group_id/members/:member_id), and R-PATH passes on it too, with the group read adding the 75 fields ofGroupDetailto the report's undeclared unsurfaced fields. The top declares that read as one the package makes for its own use, beside the LDAP link listing's, in the commit that regenerates the inventory: here the declaration would answer nothing, and R-PATH fails a declaration that answers nothing.The CE and EE scenarios compile and run at the top of the stack: the common one refuses
0and5on a project and0on a group on every surface and finds each request still pending, the CE one refuses Minimal access on the Free tier, the EE one refuses it on a subgroup and grants it on a licensed top-level group, and two probes create an Owner-level project access token and are refused an Owner-based custom role.This is the bottom layer of the stack, so the generated artifacts (the tool snapshots, the tool reference pages, the request and action records, the fine-grained table, the token footprint, the testing reference and the llms files) are regenerated once at its top. #1517 changes the same generated artifacts, so whichever of the two lands second regenerates them. The one hand-written file both change is
internal/tools/testdata/tools_meta.json, in different tool entries, and the top adds its declaration tocmd/audit_1to1/internal/paths/sent_declarations.go, which #1517 changes in other blocks.