Skip to content

Check inline comment positions past the first page of a merge request's diff, publish the pagination of mr_review.changes_get, and say when a prompt read only the first page of changed files #1584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmrplens merged 6 commits into from
Oct 29, 2026
+1,621 −277
Merged

Check inline comment positions past the first page of a merge request's diff, publish the pagination of mr_review.changes_get, and say when a prompt read only the first page of changed files #1584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f6e2a6
fix(tools): handle paginated merge request diffs
jmrplens Oct 29, 2026
c586dd5
chore(tools): give the new merge request changes test its file header
jmrplens Oct 29, 2026
9e7c3db
docs(toolutil): say again why skipping the position check is safe
jmrplens Oct 29, 2026
1a6c0c4
fix(prompts): say when a merge request changes more files than were read
jmrplens Oct 29, 2026
42944d6
chore: regenerate the testing reference
jmrplens Oct 29, 2026
46a363b
fix(prompts): withhold suggest_mr_reviewers with the merge request an…
jmrplens Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@

<!-- START TOKEN CLAIM -->

**11,396 tokens of startup context by default, the same on every GitLab tier (1,994 with `GITLAB_MCP_CAPABILITY_SURFACE=minimal`).** Two tools reach the whole catalog, where listing every tool as its own costs from 576,700 to 737,788 tokens. Measured with the cl100k_base tokenizer and checked in CI. [How it is measured](https://docs.jmrp.io/gitlab-mcp-server/tools/dynamic-tools/#how-much-startup-context-does-dynamic-mode-save)
**11,396 tokens of startup context by default, the same on every GitLab tier (1,994 with `GITLAB_MCP_CAPABILITY_SURFACE=minimal`).** Two tools reach the whole catalog, where listing every tool as its own costs from 576,797 to 737,885 tokens. Measured with the cl100k_base tokenizer and checked in CI. [How it is measured](https://docs.jmrp.io/gitlab-mcp-server/tools/dynamic-tools/#how-much-startup-context-does-dynamic-mode-save)

<!-- END TOKEN CLAIM -->

Expand Down
12 changes: 10 additions & 2 deletions cmd/audit_1to1/internal/paths/sent_declarations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,10 @@ var declaredUnsurfaced = slices.Concat([]sentDeclaration{ //nolint:gochecknoglob
"and web_url, and projectimportexport.ExportStatusOutput publishes both one level up under the same names.",
},

// The diffs the two note packages list for a position pre-check.
// The merge request the two note packages read to compare its diff_refs
// with the position, and the diffs they list for the position pre-check.
{Package: toolsDir + "/mrdiscussions", Entity: "API::Entities::MergeRequest", Field: declaredSegment, Category: categoryReadForItsOwnUse, Reason: reasonPositionSnapshot},
{Package: toolsDir + "/mrdraftnotes", Entity: "API::Entities::MergeRequest", Field: declaredSegment, Category: categoryReadForItsOwnUse, Reason: reasonPositionSnapshot},
{Package: toolsDir + "/mrdiscussions", Entity: "API::Entities::Diff", Field: declaredSegment, Category: categoryReadForItsOwnUse, Reason: reasonPositionPreCheck},
{Package: toolsDir + "/mrdraftnotes", Entity: "API::Entities::Diff", Field: declaredSegment, Category: categoryReadForItsOwnUse, Reason: reasonPositionPreCheck},

Expand Down Expand Up @@ -1201,10 +1204,15 @@ const reasonStatusCheckResponse = "POST /projects/:id/merge_requests/:merge_requ
"external_status_check.set_project_mr_status answers with a confirmation; merge_request.get returns the merge request " +
"and external_status_check.list_project_mr_checks the checks with the status each now has."

// reasonPositionSnapshot answers the merge request read for an inline position.
const reasonPositionSnapshot = "the package reads a merge request only to compare its diff_refs with the position " +
"before and after the diff lookup, and publishes nothing of that response. merge_request.get is the action " +
"that answers with the merge request."

// reasonPositionPreCheck answers the file diffs the two note packages list
// before they write a note on a line.
const reasonPositionPreCheck = "the package lists a merge request's diffs through " +
"toolutil.MergeRequestDiffsForPositionCheck only to tell a caller that a line is outside them before GitLab refuses the " +
"toolutil.MergeRequestDiffForPositionCheck only to tell a caller that a line is outside them before GitLab refuses the " +
"note with a bare 400 or 500, and returns nothing of the answer. mr_review.changes_get is the action that answers with a " +
"merge request's file diffs."

Expand Down
48 changes: 42 additions & 6 deletions docs/development/action-requests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17381,13 +17381,25 @@
],
"classic": "api",
"requests": [
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.mergeRequestPositionSnapshotMatches"
],
"sdk_methods": [
"MergeRequests.GetMergeRequest"
]
},
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid/diffs",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.MergeRequestDiffsForPositionCheck"
"toolutil.MergeRequestDiffForPositionCheck"
],
"sdk_methods": [
"MergeRequests.ListMergeRequestDiffs"
Expand All @@ -17408,7 +17420,7 @@
],
"paths": [
[
1
2
]
]
},
Expand Down Expand Up @@ -17575,13 +17587,25 @@
],
"classic": "api",
"requests": [
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.mergeRequestPositionSnapshotMatches"
],
"sdk_methods": [
"MergeRequests.GetMergeRequest"
]
},
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid/diffs",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.MergeRequestDiffsForPositionCheck"
"toolutil.MergeRequestDiffForPositionCheck"
],
"sdk_methods": [
"MergeRequests.ListMergeRequestDiffs"
Expand All @@ -17602,7 +17626,7 @@
],
"paths": [
[
1
2
]
]
},
Expand Down Expand Up @@ -17743,13 +17767,25 @@
],
"classic": "api",
"requests": [
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.mergeRequestPositionSnapshotMatches"
],
"sdk_methods": [
"MergeRequests.GetMergeRequest"
]
},
{
"kind": "rest",
"route": "GET /projects/:id/merge_requests/:merge_request_iid/diffs",
"class": "optional",
"classic": "read_api",
"sites": [
"toolutil.MergeRequestDiffsForPositionCheck"
"toolutil.MergeRequestDiffForPositionCheck"
],
"sdk_methods": [
"MergeRequests.ListMergeRequestDiffs"
Expand All @@ -17770,7 +17806,7 @@
],
"paths": [
[
1
2
]
]
},
Expand Down
33 changes: 33 additions & 0 deletions docs/development/request-inventory.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13469,12 +13469,23 @@
":merge_request_id": 1
}
},
{
"package": "internal/tools/mrdiscussions",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id",
"identifiers": {
":merge_request_id": 1,
":project_id": 1
}
},
{
"package": "internal/tools/mrdiscussions",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id/diffs",
"query": [
"page",
"per_page"
],
"identifiers": {
Expand Down Expand Up @@ -13634,12 +13645,23 @@
":project_id": 1
}
},
{
"package": "internal/tools/mrdraftnotes",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id",
"identifiers": {
":merge_request_id": 1,
":project_id": 2
}
},
{
"package": "internal/tools/mrdraftnotes",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id/diffs",
"query": [
"page",
"per_page"
],
"identifiers": {
Expand Down Expand Up @@ -20241,12 +20263,23 @@
"iid"
]
},
{
"package": "internal/toolutil",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id",
"identifiers": {
":merge_request_id": 1,
":project_id": 1
}
},
{
"package": "internal/toolutil",
"kind": "rest",
"method": "GET",
"path": "/projects/:project_id/merge_requests/:merge_request_id/diffs",
"query": [
"page",
"per_page"
],
"identifiers": {
Expand Down
30 changes: 15 additions & 15 deletions docs/development/testing/testing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@

| Metric | Value |
| ----------------------------------------------------- | -----: |
| Total test functions | 22,694 |
| Unit test functions | 21,027 |
| Total test functions | 22,715 |
| Unit test functions | 21,048 |
| E2E test functions | 1,667 |
| cmd test functions | 5,299 |
| Test files (internal/) | 741 |
| Test files (internal/) | 742 |
| Test files (cmd/) | 401 |
| Test files (test/e2e/) | 435 |
| Tool sub-packages tested | 181 |
Expand All @@ -37,22 +37,22 @@

| Pattern | Count | % |
| -------------------------------------- | -----: | ----: |
| `TestFunc_Scenario` (2-part) | 13,623 | 60.0% |
| `TestFunc_Scenario` (2-part) | 13,626 | 60.0% |
| `TestFunc` (no underscore) | 909 | 4.0% |
| `TestFunc_Scenario_Expected` (3+ part) | 8,162 | 36.0% |
| `TestFunc_Scenario_Expected` (3+ part) | 8,180 | 36.0% |

## Test Distribution

### By Layer

| Layer | Test Functions | Test Files | Description |
| ----------------------- | -------------: | ---------: | ----------------------------------------------------------------------------------------------- |
| Core packages | 4,160 | 253 | shared runtime packages such as config, GitLab client, OAuth, resources, prompts, and utilities |
| Tools orchestration | 428 | 16 | registration, meta-tool dispatch, safe mode, validation, markdown, and routing tests |
| Tool sub-packages (181) | 11,140 | 472 | domain-specific GitLab tool handlers |
| Core packages | 4,171 | 253 | shared runtime packages such as config, GitLab client, OAuth, resources, prompts, and utilities |
| Tools orchestration | 433 | 16 | registration, meta-tool dispatch, safe mode, validation, markdown, and routing tests |
| Tool sub-packages (181) | 11,145 | 473 | domain-specific GitLab tool handlers |
| E2E integration | 1,667 | 435 | build-tagged; only test/e2e/gitlab and test/e2e/orbit need a real instance |
| cmd packages | 5,299 | 401 | server entry point and developer command utilities |
| **Total** | **22,694** | **1,577** | |
| **Total** | **22,715** | **1,578** | |

### Core Packages

Expand All @@ -75,7 +75,7 @@
| mcpotel | 116 | 100.0% | Package mcpotel instruments MCP request handling with OpenTelemetry. |
| oauth | 111 | 100.0% | Package oauth provides GitLab-specific OAuth 2.0 support for HTTP mode. |
| progress | 23 | 100.0% | Package progress provides a Tracker for sending MCP progress notifications to the client during long-running tool operations. |
| prompts | 404 | 100.0% | Package prompts registers MCP prompt templates that generate AI-optimized summaries, reviews, reports, and assessments from GitLab project, group, and cross-project data. |
| prompts | 405 | 100.0% | Package prompts registers MCP prompt templates that generate AI-optimized summaries, reviews, reports, and assessments from GitLab project, group, and cross-project data. |
| resources | 240 | 100.0% | Package resources registers read-only MCP resources for GitLab and server metadata. |
| serverpool | 191 | 100.0% | Package serverpool manages a pool of credential entries keyed by GitLab token and URL. |
| sourcewalk | 9 | 100.0% | Package sourcewalk answers one question for everything here that walks this repository's tree: which directories below a walk root are this repository's own source, and which are something else that merely lives inside the checkout. |
Expand All @@ -92,8 +92,8 @@
| testutil/netfixture | 8 | 100.0% | Package netfixture gives a test an HTTP address that stops answering without giving its port away. |
| testutil/serialtypecheck | 6 | 100.0% | Package serialtypecheck makes golang.org/x/tools/go/packages type-check one package at a time in a race build, and does nothing in any other build. |
| testutil/shardio | 32 | 100.0% | Package shardio is the shard mechanism the records written by a test process and read back by a command are built on: one shard file per process, one JSON line per record, a directory tree read in one pass, and a line nobody can read reported rather than dropped. |
| toolutil | 1,349 | 100.0% | Package toolutil provides shared utilities for MCP tool handler sub-packages. |
| **Subtotal** | **4,160** | | |
| toolutil | 1,359 | 100.0% | Package toolutil provides shared utilities for MCP tool handler sub-packages. |
| **Subtotal** | **4,171** | | |

### Tool Sub-Packages (Top Domains by Test Count)

Expand Down Expand Up @@ -246,10 +246,10 @@
| modelregistry | 13 | 3 | 100.0% | 1 |
| mrapprovals | 108 | 3 | 100.0% | 7 |
| mrapprovalsettings | 14 | 4 | 100.0% | 4 |
| mrchanges | 43 | 1 | 100.0% | 4 |
| mrchanges | 47 | 2 | 100.0% | 4 |
| mrcontextcommits | 40 | 3 | 100.0% | 3 |
| mrdiscussions | 67 | 1 | 100.0% | 7 |
| mrdraftnotes | 80 | 3 | 100.0% | 7 |
| mrdraftnotes | 81 | 3 | 100.0% | 7 |
| mrnotes | 52 | 4 | 100.0% | 5 |
| namespaces | 52 | 2 | 100.0% | 4 |
| notifications | 40 | 2 | 100.0% | 6 |
Expand Down Expand Up @@ -313,7 +313,7 @@
| wikis | 74 | 2 | 100.0% | 6 |
| workitems | 141 | 6 | 100.0% | 6 |
| workitemsavedviews | 63 | 4 | 100.0% | 7 |
| **Total** | **11,140** | **472** | | **1,195** |
| **Total** | **11,145** | **473** | | **1,195** |

</details>

Expand Down
Loading
Loading